Loading ...
Sorry, an error occurred while loading the content.

Re: patch to fix memory leak (when ex file completion results in no match)

Expand Messages
  • Bram Moolenaar
    ... I don t think this helps, because I can t find a combination of WILD_FREE being used with the third argument not being NULL. ... I don t think this helps.
    Message 1 of 3 , Sep 30 9:05 AM
    • 0 Attachment
      Dominique Pelle wrote:

      > I can reproduce a memory leak 100% of the time in vim-7.1.123:
      >
      > Start vim with:
      >
      > vim -u NONE -U NONE
      >
      > Then set the following options:
      >
      > :set nocompatible
      > :set wildmode=longest,list
      > :set wildmenu
      >
      > vim will then leak memory whenever file completion on Ex command
      > results in no match:
      >
      > :e /<TAB> # OK no leak (file completion lists some files
      > # /usr, /tmp, /dev, etc)
      >
      > :e /fff<TAB> # leaks 1 block of memory! (assuming no filename start
      > # with /fff)
      >
      > Here is stack of the leak reported by valgrind (when exiting vim):
      >
      > ==12117== 21 bytes in 4 blocks are definitely lost in loss record 2 of 15
      > ==12117== at 0x4021620: malloc (vg_replace_malloc.c:149)
      > ==12117== by 0x8102F1C: lalloc (misc2.c:857)
      > ==12117== by 0x8102E3E: alloc (misc2.c:756)
      > ==12117== by 0x81032C9: vim_strnsave (misc2.c:1155)
      > ==12117== by 0x80B00F9: nextwild (ex_getln.c:3254)
      > ==12117== by 0x80AC44B: getcmdline (ex_getln.c:793)
      > ==12117== by 0x80AE69B: getexline (ex_getln.c:2078)
      > ==12117== by 0x8098E80: do_cmdline (ex_docmd.c:995)
      > ==12117== by 0x8116F0F: nv_colon (normal.c:5168)
      > ==12117== by 0x8111043: normal_cmd (normal.c:1141)
      > ==12117== by 0x80D7D22: main_loop (main.c:1181)
      > ==12117== by 0x80D7936: main (main.c:940)
      >
      > Here is the relevant code where memory is allocated:
      >
      > ex_getln.c:
      >
      > 3254 p2 = ExpandOne(xp, p1, vim_strnsave(&ccline.cmdbuff[i], oldlen),
      > 3255 WILD_HOME_REPLACE|WILD_ADD_SLASH|WILD_SILENT|WILD_ESCAPE
      > 3256 |options, type);
      >
      > Function vim_strnsave() allocates a block of memory and passes
      > it to function ExpanOne (3rd argument).
      >
      > Adding some printf(), I can see that function ExpandOne()
      > returns without freeing its 3rd argument at line 3414:
      >
      > misc2.c:
      >
      > 3413 if (mode == WILD_FREE) /* only release file name */
      > 3414 return NULL;
      >
      > I think function ExpandOne() should free its 3rd argument
      > before returning at line 3414.

      I don't think this helps, because I can't find a combination of
      WILD_FREE being used with the third argument not being NULL.

      > There are also 3 other lines in ExpandOne() where it can potentially
      > return without freeing its 3rd argument at lines 3396, 3397 and 3400
      > (though I am not sure whether they can happen in practice). I did
      > not fix them but they could possibly be other leaks.
      >
      > I attach a patch which fixes the leak at line 3414 which I can
      > reproduce as explained above.

      I don't think this helps. The normal return at the end of the function
      may leak memory when "orig" was not saved in "orig_save". This patch
      should fix the leak:


      *** ../../vim-7.1.126/src/ex_getln.c Sat Sep 29 14:15:00 2007
      --- ex_getln.c Sun Sep 30 17:55:47 2007
      ***************
      *** 3540,3545 ****
      --- 3544,3553 ----

      if (mode == WILD_EXPAND_FREE || mode == WILD_ALL)
      ExpandCleanup(xp);
      +
      + /* Free "orig" if it wasn't stored in "orig_save". */
      + if (orig != orig_save)
      + vim_free(orig);

      return ss;
      }


      Please verify this fixes the leak, also without your suggested patch.
      I tried your example and it appears to fix it. But I have not tried
      every possible completion.

      > Note that it's OK to have one block reported as leak (which is
      > actually not a leak because there is a static var orig_save
      > to cache I think) but it's not OK to leak more than one block.

      I thought that valgrind does check static variables and doesn't report a
      leak when there is a pointer that uses the allocated memory.

      > I'm using vim-7.1 (patches 1-123), on Linux, built with
      > 'configure --with-features=huge'
      >
      > I built vim by uncommenting out "PROFILE_CFLAGS = -DEXITFREE"
      > in vim7/src/Makefile to help find leaks. After uncommenting that line,
      > I had a compilation error (clip_exclude_prog() not declared):
      >
      > gcc -c -I. -Iproto -DHAVE_CONFIG_H -g -O0 -Wall
      > -Wmissing-prototypes -DEXITFREE -o objects/misc2.o misc2.c
      > misc2.c: In function 'free_all_mem':
      > misc2.c:1040: error: 'clip_exclude_prog' undeclared (first use in this function)
      > misc2.c:1040: error: (Each undeclared identifier is reported only once
      > misc2.c:1040: error: for each function it appears in.)
      >
      > I worked around that error by moving clip_exclude_prog() declaration out
      > of #ifdef FEAT_CLIPBOARD ... #endif. Should that be fixed too or is
      > there a better way to define EXITFREE in the build?

      I don't check building with EXITFREE and various features. I think the
      right fix is to put that line in free_all_mem() inside #ifdef
      FEAT_CLIPBOARD:

      *** /home/mool/vim/vim-7.1.126/src/misc2.c Sat Sep 29 14:15:00 2007
      --- misc2.c Sun Sep 30 18:00:09 2007
      ***************
      *** 1037,1043 ****
      --- 1038,1046 ----

      /* Free some global vars. */
      vim_free(username);
      + # ifdef FEAT_CLIPBOARD
      vim_free(clip_exclude_prog);
      + # endif
      vim_free(last_cmdline);
      vim_free(new_last_cmdline);
      set_keep_msg(NULL, 0);

      --
      Mushrooms always grow in damp places and so they look like umbrellas.

      /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
      /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
      \\\ download, build and distribute -- http://www.A-A-P.org ///
      \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

      --~--~---------~--~----~------------~-------~--~----~
      You received this message from the "vim_dev" maillist.
      For more information, visit http://www.vim.org/maillist.php
      -~----------~----~----~----~------~----~------~--~---
    Your message has been successfully submitted and would be delivered to recipients shortly.