GNU bug report logs - #9830
23.3.50; y-or-n-p-with-timeout with GUI leaks memory

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 9830 in the body.
You can then email your comments to 9830 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: YAMAMOTO Mitsuharu <mituharu <at> math.s.chiba-u.ac.jp>
To: bug-gnu-emacs <at> gnu.org
Subject: 23.3.50; y-or-n-p-with-timeout with GUI leaks memory
Date: Sat, 22 Oct 2011 13:35:27 +0900

Currently, xdialog_show in xmenu.c calls
free_menubar_widget_value_tree after calling create_and_show_dialog:

  /* Actually create and show the dialog.  */
  create_and_show_dialog (f, first_wv);

  /* Free the widget_value objects we used to specify the contents.  */
  free_menubar_widget_value_tree (first_wv);

But create_and_show_dialog may longjmp in the following case:

  (let (last-nonmenu-event)
    (y-or-n-p-with-timeout "test" 3 'default))
  ;; wait 3 seconds.

In this case, free_menubar_widget_value_tree is not called and the
memory allocated for `first_wv' is leaked.

The patch below uses record_unwind_protect as usual for avoiding this
memory leak.  I think xmenu_show has the same problem.  I'm not sure
about the W32 case.

				     YAMAMOTO Mitsuharu
				mituharu <at> math.s.chiba-u.ac.jp

=== modified file 'src/xmenu.c'
*** src/xmenu.c	2011-05-09 11:13:02 +0000
--- src/xmenu.c	2011-10-22 02:14:19 +0000
***************
*** 1658,1663 ****
--- 1658,1674 ----
  
  #endif /* not USE_GTK */
  
+ static Lisp_Object
+ cleanup_widget_value_tree (Lisp_Object arg)
+ {
+   struct Lisp_Save_Value *p = XSAVE_VALUE (arg);
+   widget_value *wv = p->pointer;
+ 
+   free_menubar_widget_value_tree (wv);
+ 
+   return Qnil;
+ }
+ 
  Lisp_Object
  xmenu_show (FRAME_PTR f, int x, int y, int for_click, int keymaps,
  	    Lisp_Object title, char **error, EMACS_UINT timestamp)
***************
*** 1672,1677 ****
--- 1683,1690 ----
  
    int first_pane;
  
+   int specpdl_count = SPECPDL_INDEX ();
+ 
    if (! FRAME_X_P (f))
      abort ();
  
***************
*** 1866,1876 ****
    /* No selection has been chosen yet.  */
    menu_item_selection = 0;
  
    /* Actually create and show the menu until popped down.  */
    create_and_show_popup_menu (f, first_wv, x, y, for_click, timestamp);
  
!   /* Free the widget_value objects we used to specify the contents.  */
!   free_menubar_widget_value_tree (first_wv);
  
    /* Find the selected item, and its pane, to return
       the proper value.  */
--- 1879,1893 ----
    /* No selection has been chosen yet.  */
    menu_item_selection = 0;
  
+   /* Make sure to free the widget_value objects we used to specify the
+      contents even with longjmp.  */
+   record_unwind_protect (cleanup_widget_value_tree,
+ 			 make_save_value (first_wv, 0));
+ 
    /* Actually create and show the menu until popped down.  */
    create_and_show_popup_menu (f, first_wv, x, y, for_click, timestamp);
  
!   unbind_to (specpdl_count, Qnil);
  
    /* Find the selected item, and its pane, to return
       the proper value.  */
***************
*** 2064,2069 ****
--- 2081,2088 ----
    /* 1 means we've seen the boundary between left-hand elts and right-hand.  */
    int boundary_seen = 0;
  
+   int specpdl_count = SPECPDL_INDEX ();
+ 
    if (! FRAME_X_P (f))
      abort ();
  
***************
*** 2177,2187 ****
    /* No selection has been chosen yet.  */
    menu_item_selection = 0;
  
    /* Actually create and show the dialog.  */
    create_and_show_dialog (f, first_wv);
  
!   /* Free the widget_value objects we used to specify the contents.  */
!   free_menubar_widget_value_tree (first_wv);
  
    /* Find the selected item, and its pane, to return
       the proper value.  */
--- 2196,2210 ----
    /* No selection has been chosen yet.  */
    menu_item_selection = 0;
  
+   /* Make sure to free the widget_value objects we used to specify the
+      contents even with longjmp.  */
+   record_unwind_protect (cleanup_widget_value_tree,
+ 			 make_save_value (first_wv, 0));
+ 
    /* Actually create and show the dialog.  */
    create_and_show_dialog (f, first_wv);
  
!   unbind_to (specpdl_count, Qnil);
  
    /* Find the selected item, and its pane, to return
       the proper value.  */



In GNU Emacs 23.3.50.1 (x86_64-apple-darwin10.8.0, GTK+ Version 2.24.6)
 of 2011-10-20 on yamamoto-mitsuharu-no-iMac.local
Windowing system distributor `The X.Org Foundation', version 11.0.10402000
configured using `configure  '--enable-checking' 'LDFLAGS=-L/opt/local/lib' 'CPPFLAGS=-I/opt/local/include' 'CFLAGS=-g''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: ja_JP.UTF-8
  value of $XMODIFIERS: nil
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

Major mode: Fundamental

Minor modes in effect:
  tooltip-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  blink-cursor-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Message #8 received at 9830 <at> debbugs.gnu.org (full text, mbox):

From: Jan Djärv <jan.h.d <at> swipnet.se>
To: YAMAMOTO Mitsuharu <mituharu <at> math.s.chiba-u.ac.jp>
Cc: 9830 <at> debbugs.gnu.org
Subject: Re: bug#9830: 23.3.50; y-or-n-p-with-timeout with GUI leaks memory
Date: Sun, 30 Oct 2011 18:41:40 +0100

Hello.

22 okt 2011 kl. 06:35 skrev YAMAMOTO Mitsuharu:

> Currently, xdialog_show in xmenu.c calls
> free_menubar_widget_value_tree after calling create_and_show_dialog:
> 
>  /* Actually create and show the dialog.  */
>  create_and_show_dialog (f, first_wv);
> 
>  /* Free the widget_value objects we used to specify the contents.  */
>  free_menubar_widget_value_tree (first_wv);
> 
> But create_and_show_dialog may longjmp in the following case:
> 
>  (let (last-nonmenu-event)
>    (y-or-n-p-with-timeout "test" 3 'default))
>  ;; wait 3 seconds.
> 
> In this case, free_menubar_widget_value_tree is not called and the
> memory allocated for `first_wv' is leaked.
> 
> The patch below uses record_unwind_protect as usual for avoiding this
> memory leak.  I think xmenu_show has the same problem.  I'm not sure
> about the W32 case.
> 

You are correct.  Can you install the patch?
As for xmenu_show, is there a case where a menu can be interrupted like this?

	Jan D.


> 				     YAMAMOTO Mitsuharu
> 				mituharu <at> math.s.chiba-u.ac.jp
> 
> === modified file 'src/xmenu.c'
> *** src/xmenu.c	2011-05-09 11:13:02 +0000
> --- src/xmenu.c	2011-10-22 02:14:19 +0000
> ***************
> *** 1658,1663 ****
> --- 1658,1674 ----
> 
>  #endif /* not USE_GTK */
> 
> + static Lisp_Object
> + cleanup_widget_value_tree (Lisp_Object arg)
> + {
> +   struct Lisp_Save_Value *p = XSAVE_VALUE (arg);
> +   widget_value *wv = p->pointer;
> + 
> +   free_menubar_widget_value_tree (wv);
> + 
> +   return Qnil;
> + }
> + 
>  Lisp_Object
>  xmenu_show (FRAME_PTR f, int x, int y, int for_click, int keymaps,
>  	    Lisp_Object title, char **error, EMACS_UINT timestamp)
> ***************
> *** 1672,1677 ****
> --- 1683,1690 ----
> 
>    int first_pane;
> 
> +   int specpdl_count = SPECPDL_INDEX ();
> + 
>    if (! FRAME_X_P (f))
>      abort ();
> 
> ***************
> *** 1866,1876 ****
>    /* No selection has been chosen yet.  */
>    menu_item_selection = 0;
> 
>    /* Actually create and show the menu until popped down.  */
>    create_and_show_popup_menu (f, first_wv, x, y, for_click, timestamp);
> 
> !   /* Free the widget_value objects we used to specify the contents.  */
> !   free_menubar_widget_value_tree (first_wv);
> 
>    /* Find the selected item, and its pane, to return
>       the proper value.  */
> --- 1879,1893 ----
>    /* No selection has been chosen yet.  */
>    menu_item_selection = 0;
> 
> +   /* Make sure to free the widget_value objects we used to specify the
> +      contents even with longjmp.  */
> +   record_unwind_protect (cleanup_widget_value_tree,
> + 			 make_save_value (first_wv, 0));
> + 
>    /* Actually create and show the menu until popped down.  */
>    create_and_show_popup_menu (f, first_wv, x, y, for_click, timestamp);
> 
> !   unbind_to (specpdl_count, Qnil);
> 
>    /* Find the selected item, and its pane, to return
>       the proper value.  */
> ***************
> *** 2064,2069 ****
> --- 2081,2088 ----
>    /* 1 means we've seen the boundary between left-hand elts and right-hand.  */
>    int boundary_seen = 0;
> 
> +   int specpdl_count = SPECPDL_INDEX ();
> + 
>    if (! FRAME_X_P (f))
>      abort ();
> 
> ***************
> *** 2177,2187 ****
>    /* No selection has been chosen yet.  */
>    menu_item_selection = 0;
> 
>    /* Actually create and show the dialog.  */
>    create_and_show_dialog (f, first_wv);
> 
> !   /* Free the widget_value objects we used to specify the contents.  */
> !   free_menubar_widget_value_tree (first_wv);
> 
>    /* Find the selected item, and its pane, to return
>       the proper value.  */
> --- 2196,2210 ----
>    /* No selection has been chosen yet.  */
>    menu_item_selection = 0;
> 
> +   /* Make sure to free the widget_value objects we used to specify the
> +      contents even with longjmp.  */
> +   record_unwind_protect (cleanup_widget_value_tree,
> + 			 make_save_value (first_wv, 0));
> + 
>    /* Actually create and show the dialog.  */
>    create_and_show_dialog (f, first_wv);
> 
> !   unbind_to (specpdl_count, Qnil);
> 
>    /* Find the selected item, and its pane, to return
>       the proper value.  */
> 
> 
> 
> In GNU Emacs 23.3.50.1 (x86_64-apple-darwin10.8.0, GTK+ Version 2.24.6)
> of 2011-10-20 on yamamoto-mitsuharu-no-iMac.local
> Windowing system distributor `The X.Org Foundation', version 11.0.10402000
> configured using `configure  '--enable-checking' 'LDFLAGS=-L/opt/local/lib' 'CPPFLAGS=-I/opt/local/include' 'CFLAGS=-g''
> 
> Important settings:
>  value of $LC_ALL: nil
>  value of $LC_COLLATE: nil
>  value of $LC_CTYPE: nil
>  value of $LC_MESSAGES: nil
>  value of $LC_MONETARY: nil
>  value of $LC_NUMERIC: nil
>  value of $LC_TIME: nil
>  value of $LANG: ja_JP.UTF-8
>  value of $XMODIFIERS: nil
>  locale-coding-system: utf-8-unix
>  default enable-multibyte-characters: t
> 
> Major mode: Fundamental
> 
> Minor modes in effect:
>  tooltip-mode: t
>  mouse-wheel-mode: t
>  tool-bar-mode: t
>  menu-bar-mode: t
>  file-name-shadow-mode: t
>  blink-cursor-mode: t
>  auto-encryption-mode: t
>  auto-compression-mode: t
>  line-number-mode: t
>  transient-mark-mode: t
> 
> 
> 

Message #13 received at 9830-done <at> debbugs.gnu.org (full text, mbox):

From: YAMAMOTO Mitsuharu <mituharu <at> math.s.chiba-u.ac.jp>
To: 9830-done <at> debbugs.gnu.org
Subject: Re: bug#9830: 23.3.50; y-or-n-p-with-timeout with GUI leaks memory
Date: Mon, 31 Oct 2011 12:21:14 +0900

>>>>> On Sun, 30 Oct 2011 18:41:40 +0100, Jan Djärv <jan.h.d <at> swipnet.se> said:

>> The patch below uses record_unwind_protect as usual for avoiding
>> this memory leak.  I think xmenu_show has the same problem.  I'm
>> not sure about the W32 case.
>> 

> You are correct.  Can you install the patch?

Done.

> As for xmenu_show, is there a case where a menu can be interrupted
> like this?

Yes.  For example,

  (with-timeout (3 'default)
    (x-popup-menu t '("Title" ("foo" "nonselectable"))))

Actually, y-or-n-p-with-timeout is implemented using with-timeout.

				     YAMAMOTO Mitsuharu
				mituharu <at> math.s.chiba-u.ac.jp

Message #16 received at 9830 <at> debbugs.gnu.org (full text, mbox):

From: Jan Djärv <jan.h.d <at> swipnet.se>
To: YAMAMOTO Mitsuharu <mituharu <at> math.s.chiba-u.ac.jp>
Cc: 9830 <at> debbugs.gnu.org
Subject: Re: bug#9830: 23.3.50; y-or-n-p-with-timeout with GUI leaks memory
Date: Sun, 6 Nov 2011 16:54:52 +0100

Hello.

31 okt 2011 kl. 04:21 skrev YAMAMOTO Mitsuharu:

>>>>>> On Sun, 30 Oct 2011 18:41:40 +0100, Jan Djärv <jan.h.d <at> swipnet.se> said:
> 
>>> The patch below uses record_unwind_protect as usual for avoiding
>>> this memory leak.  I think xmenu_show has the same problem.  I'm
>>> not sure about the W32 case.
>>> 
> 
>> You are correct.  Can you install the patch?
> 
> Done.

I see you checked in this in the emacs-23 tree.  Is that still merged to the trunk?

> 
>> As for xmenu_show, is there a case where a menu can be interrupted
>> like this?
> 
> Yes.  For example,
> 
>  (with-timeout (3 'default)
>    (x-popup-menu t '("Title" ("foo" "nonselectable"))))
> 
> Actually, y-or-n-p-with-timeout is implemented using with-timeout.

I'm waiting for your fix to show up in the trunk before fixing this.  I don't think it makes sense to use the emacs-23 tree anymore.

	Jan D.

Message #19 received at 9830 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Jan Djärv <jan.h.d <at> swipnet.se>
Cc: 9830 <at> debbugs.gnu.org, YAMAMOTO Mitsuharu <mituharu <at> math.s.chiba-u.ac.jp>
Subject: Re: bug#9830: 23.3.50; y-or-n-p-with-timeout with GUI leaks memory
Date: Sun, 06 Nov 2011 18:39:04 -0500

Jan Djärv wrote:

> I'm waiting for your fix to show up in the trunk before fixing this.

I merged it.

>  I don't think it makes sense to use the emacs-23 tree anymore.

I agree (in fact I think this point was reached months ago).

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.