On Sun, Jun 8, 2008 at 2:25 PM, Bram Moolenaar <[EMAIL PROTECTED]> wrote:
> Dominique Pelle wrote:
>
>> On Sat, Jun 7, 2008 at 8:44 AM, Dominique Pelle wrote:
>>
>> > Attached:
>> > - crash-os_unix.c.patch
>> > - leak-os_unix.c.patch
>>
>> I built the gui-motif, gui-athena and gui-gtk2 on Linux x86 and the crash in
>> os_unix.c only happens with gui-motif (I'm using LessTif).
>>
>> So instead of doing #if 0 as in my previous patch, it should be better to
>> do #ifndef FEAT_GUI_MOTIF.
>>
>> gui-athena shows errors with -DEXITFREE just a few lines below though:
>>
>> ==15738== Invalid read of size 4
>> ==15738== at 0x42CDEE7: XCloseDisplay (in /usr/lib/libX11.so.6.2.0)
>> ==15738== by 0x8140419: mch_free_mem (os_unix.c:2900)
>> ==15738== by 0x810A969: free_all_mem (misc2.c:1125)
>> ==15738== by 0x8140573: mch_exit (os_unix.c:3012)
>> ==15738== by 0x80DE4C8: getout (main.c:1342)
>> ==15738== by 0x80A56C3: ex_quit_all (ex_docmd.c:6281)
>> ==15738== by 0x809F8CB: do_one_cmd (ex_docmd.c:2623)
>> ==15738== by 0x809D18C: do_cmdline (ex_docmd.c:1099)
>> ==15738== by 0x812030E: nv_colon (normal.c:5179)
>> ==15738== by 0x8119B88: normal_cmd (normal.c:1152)
>> ==15738== by 0x80DE20A: main_loop (main.c:1181)
>> ==15738== by 0x80DDD5A: main (main.c:940)
>> ==15738== Address 0x44020cc is 148 bytes inside a block of size 1,340 free'd
>> ==15738== at 0x402265C: free (vg_replace_malloc.c:323)
>> ==15738== by 0x42E0F3E: _XFreeDisplayStructure (in
>> /usr/lib/libX11.so.6.2.0)
>> ==15738== by 0x42CDFC5: XCloseDisplay (in /usr/lib/libX11.so.6.2.0)
>> ==15738== by 0x424EC49: (within /usr/lib/libXt.so.6.0.0)
>> ==15738== by 0x424EE36: XtCloseDisplay (in /usr/lib/libXt.so.6.0.0)
>> ==15738== by 0x424EE70: (within /usr/lib/libXt.so.6.0.0)
>> ==15738== by 0x424F164: XtDestroyApplicationContext (in
>> /usr/lib/libXt.so.6.0.0)
>> ==15738== by 0x81403F4: mch_free_mem (os_unix.c:2896)
>> ==15738== by 0x810A969: free_all_mem (misc2.c:1125)
>> ==15738== by 0x8140573: mch_exit (os_unix.c:3012)
>> ==15738== by 0x80DE4C8: getout (main.c:1342)
>> ==15738== by 0x80A56C3: ex_quit_all (ex_docmd.c:6281)
>> ==15738== by 0x809F8CB: do_one_cmd (ex_docmd.c:2623)
>> ==15738== by 0x809D18C: do_cmdline (ex_docmd.c:1099)
>> ==15738== by 0x812030E: nv_colon (normal.c:5179)
>> ==15738== by 0x8119B88: normal_cmd (normal.c:1152)
>> ==15738== by 0x80DE20A: main_loop (main.c:1181)
>> ==15738== by 0x80DDD5A: main (main.c:940)
>>
>> os_unix.c:
>>
>> 2890 # if (defined(FEAT_X11) && defined(FEAT_XCLIPBOARD)) || defined(PROTO)
>> 2891 if (xterm_Shell != (Widget)0)
>> 2892 XtDestroyWidget(xterm_Shell);
>> 2893 if (xterm_dpy != NULL)
>> 2894 XtCloseDisplay(xterm_dpy);
>> 2895 if (app_context != (XtAppContext)NULL)
>> !! 2896 XtDestroyApplicationContext(app_context);
>> 2897 # endif
>> 2898 # ifdef FEAT_X11
>> 2899 if (x11_display != NULL && x11_display != xterm_dpy)
>> !! 2900 XCloseDisplay(x11_display);
>> 2901 # endif
>>
>> Line 2900 is using memory which was freed at line 2896
>> according to valgrind. So it seems that
>> XtDestroyApplicationContext(app_context) is also
>> destroying x11_display.
>>
>> Attached patch fixes the error.
>
> Thanks. I wonder if we can use #ifndef LESSTIF_VERSION instead of
> FEAT_GUI_MOTIF. The other "#if 0" suggests that Solaris also has this
> problem. But I'm not sure about this specific code, since it was added
> much later.
Yes, #ifndef LESSTIF_VERSION is better than #ifndef FEAT_GUI_MOTIF.
I tested it with both lesstif and Open Motif, it's fine. After the attached
patch, neither lesstif, nor Open Motif access freed memory.
Open Motif needs the same fix as for athena gui i.e. set x11_display
to NULL after 'XtDestroyApplicationContext(app_context);' since it
seems to take care of already calling 'XCloseDisplay(x11_display)'.
Lesstif and Open Motif also use uninitialized memory but I assume
that's a problem in those libs and not with Vim.
There are several things not freed with all GUIs (athena, open motif
lesstif, gtk2, gnome2) when exiting even with -DEXITFREE but that's
not a major problem I think. I fixed at least the following leak which
happens at least for Motif and athena gui (patch: leak-gui_x11.c).
==23613== 32 bytes in 1 blocks are definitely lost in loss record 34 of 126
==23613== at 0x4022AB8: malloc (vg_replace_malloc.c:207)
==23613== by 0x40A8C08: xpmParseDataAndCreate (in /usr/lib/libXm.so.2.0.1)
==23613== by 0x40AA0A6: XpmCreateImageFromData (in /usr/lib/libXm.so.2.0.1)
==23613== by 0x40AA342: XpmCreatePixmapFromData (in /usr/lib/libXm.so.2.0.1)
==23613== by 0x81C9E0E: gui_mch_init (gui_x11.c:1541)
==23613== by 0x81BA6F1: gui_init (gui.c:457)
==23613== by 0x81A56BD: set_termname (term.c:1845)
==23613== by 0x81A609D: termcapinit (term.c:2514)
==23613== by 0x81BA0BE: gui_start (gui.c:91)
==23613== by 0x80DED45: main (main.c:634)
-- Dominique
--~--~---------~--~----~------------~-------~--~----~
You received this message from the "vim_dev" maillist.
For more information, visit http://www.vim.org/maillist.php
-~----------~----~----~----~------~----~------~--~---
Index: gui_x11.c
===================================================================
RCS file: /cvsroot/vim/vim7/src/gui_x11.c,v
retrieving revision 1.18
diff -c -r1.18 gui_x11.c
*** gui_x11.c 10 May 2007 19:11:45 -0000 1.18
--- gui_x11.c 8 Jun 2008 14:07:11 -0000
***************
*** 1538,1545 ****
--- 1538,1548 ----
attr.depth = DefaultDepthOfScreen(scr);
if (!icon)
+ {
XpmCreatePixmapFromData(dsp, root_window, magick, &icon,
&icon_mask, &attr);
+ XpmFreeAttributes(&attr);
+ }
# ifdef FEAT_GUI_ATHENA
XtVaSetValues(vimShell, XtNiconPixmap, icon, XtNiconMask, icon_mask, NULL);
Index: os_unix.c
===================================================================
RCS file: /cvsroot/vim/vim7/src/os_unix.c,v
retrieving revision 1.80
diff -c -r1.80 os_unix.c
*** os_unix.c 7 May 2008 17:07:38 -0000 1.80
--- os_unix.c 8 Jun 2008 14:08:49 -0000
***************
*** 209,214 ****
--- 209,215 ----
{
SmcConn smcconn; /* The SM connection ID */
IceConn iceconn; /* The ICE connection ID */
+ char *clientid; /* The client ID for the current session */
Bool save_yourself; /* If we're in the middle of a save_yourself */
Bool shutdown; /* If we're in shutdown mode */
} xsmp_config_T;
***************
*** 2890,2899 ****
--- 2891,2906 ----
# if (defined(FEAT_X11) && defined(FEAT_XCLIPBOARD)) || defined(PROTO)
if (xterm_Shell != (Widget)0)
XtDestroyWidget(xterm_Shell);
+ # ifndef LESSTIF_VERSION
+ /* Lesstif crashes here, lose some memory */
if (xterm_dpy != NULL)
XtCloseDisplay(xterm_dpy);
if (app_context != (XtAppContext)NULL)
+ {
XtDestroyApplicationContext(app_context);
+ x11_display = NULL; /* freed by XtDestroyApplicationContext() */
+ }
+ # endif
# endif
# ifdef FEAT_X11
if (x11_display != NULL && x11_display != xterm_dpy)
***************
*** 6557,6563 ****
xsmp_init(void)
{
char errorstring[80];
- char *clientid;
SmcCallbacks smcallbacks;
#if 0
SmPropValue smname;
--- 6564,6569 ----
***************
*** 6599,6605 ****
| SmcSaveCompleteProcMask | SmcShutdownCancelledProcMask,
&smcallbacks,
NULL,
! &clientid,
sizeof(errorstring),
errorstring);
if (xsmp.smcconn == NULL)
--- 6605,6611 ----
| SmcSaveCompleteProcMask | SmcShutdownCancelledProcMask,
&smcallbacks,
NULL,
! &xsmp.clientid,
sizeof(errorstring),
errorstring);
if (xsmp.smcconn == NULL)
***************
*** 6638,6643 ****
--- 6644,6651 ----
if (xsmp_icefd != -1)
{
SmcCloseConnection(xsmp.smcconn, 0, NULL);
+ vim_free(xsmp.clientid);
+ xsmp.clientid = NULL;
xsmp_icefd = -1;
}
}
