* bug#9830: 23.3.50; y-or-n-p-with-timeout with GUI leaks memory
@ 2011-10-22 4:35 YAMAMOTO Mitsuharu
2011-10-30 17:41 ` Jan Djärv
0 siblings, 1 reply; 5+ messages in thread
From: YAMAMOTO Mitsuharu @ 2011-10-22 4:35 UTC (permalink / raw)
To: 9830
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@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
^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#9830: 23.3.50; y-or-n-p-with-timeout with GUI leaks memory
2011-10-22 4:35 bug#9830: 23.3.50; y-or-n-p-with-timeout with GUI leaks memory YAMAMOTO Mitsuharu
@ 2011-10-30 17:41 ` Jan Djärv
2011-10-31 3:21 ` YAMAMOTO Mitsuharu
0 siblings, 1 reply; 5+ messages in thread
From: Jan Djärv @ 2011-10-30 17:41 UTC (permalink / raw)
To: YAMAMOTO Mitsuharu; +Cc: 9830
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@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
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#9830: 23.3.50; y-or-n-p-with-timeout with GUI leaks memory
2011-10-30 17:41 ` Jan Djärv
@ 2011-10-31 3:21 ` YAMAMOTO Mitsuharu
2011-11-06 15:54 ` Jan Djärv
0 siblings, 1 reply; 5+ messages in thread
From: YAMAMOTO Mitsuharu @ 2011-10-31 3:21 UTC (permalink / raw)
To: 9830-done
>>>>> On Sun, 30 Oct 2011 18:41:40 +0100, Jan Djärv <jan.h.d@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@math.s.chiba-u.ac.jp
^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#9830: 23.3.50; y-or-n-p-with-timeout with GUI leaks memory
2011-10-31 3:21 ` YAMAMOTO Mitsuharu
@ 2011-11-06 15:54 ` Jan Djärv
2011-11-06 23:39 ` Glenn Morris
0 siblings, 1 reply; 5+ messages in thread
From: Jan Djärv @ 2011-11-06 15:54 UTC (permalink / raw)
To: YAMAMOTO Mitsuharu; +Cc: 9830
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@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.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-11-06 23:39 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-22 4:35 bug#9830: 23.3.50; y-or-n-p-with-timeout with GUI leaks memory YAMAMOTO Mitsuharu
2011-10-30 17:41 ` Jan Djärv
2011-10-31 3:21 ` YAMAMOTO Mitsuharu
2011-11-06 15:54 ` Jan Djärv
2011-11-06 23:39 ` Glenn Morris
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).