GNU bug report logs - #15802
24.3.50; PUSH_HANDLER leaks memory?

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: tracker <at> debbugs.gnu.org
Subject: bug#15802: closed (24.3.50; PUSH_HANDLER leaks memory?)
Date: Tue, 05 Nov 2013 16:29:02 +0000

[Message part 1 (text/plain, inline)]

Your message dated Tue, 05 Nov 2013 11:28:00 -0500
with message-id <jwv1u2uc0of.fsf-monnier+emacsbugs <at> gnu.org>
and subject line Re: bug#15802: 24.3.50; PUSH_HANDLER leaks memory?
has caused the debbugs.gnu.org bug report #15802,
regarding 24.3.50; PUSH_HANDLER leaks memory?
to be marked as done.

(If you believe you have received this mail in error, please contact
help-debbugs <at> gnu.org.)


-- 
15802: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15802
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

From: Jan Djärv <jan.h.d <at> me.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.3.50; PUSH_HANDLER leaks memory?
Date: Mon, 04 Nov 2013 19:32:39 +0100

Hello.

While running leaks on OSX, it indicated that PUSH_HANDLER.  I can't find anywhere where
handlerlist members are freed.

Consider handlerlist = NULL.
One PUSH_HANDLER will then allocate one struct handler that becomes handlerlist, next is NULL and nextfree is NULL.

In the code, for example internal_condition_case_1, the code simply does
    handlerlist = handlerllist->next

thus loosing the allocated struct.  There must be a corresponding xfree somewhere but I can't find it.

	Jan D.


In GNU Emacs 24.3.50.1 (x86_64-apple-darwin13.0.0, NS apple-appkit-1265.00)
of 2013-11-04 on zeplin
Bzr revision: 114945 jan.h.d <at> swipnet.se-20131104175717-8j1fzo686naq33js
Windowing system distributor `Apple', version 10.3.1265
Configured using:
`configure --verbose --with-ns CFLAGS=-g3'

Important settings:
  value of $LC_COLLATE: C
  value of $LANG: sv_SE.UTF-8
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

Major mode: Lisp Interaction

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

Recent input:
<escape> x r e p o r t - e m <tab> <return>

Recent messages:
Unable to load color "unspecified-fg"
For information about GNU Emacs and the GNU system, type C-h C-a.
Unable to load color "unspecified-fg" [2 times]

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message format-spec rfc822 mml
easymenu mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils time-date tooltip ediff-hook vc-hooks
lisp-float-type mwheel ns-win tool-bar dnd fontset image regexp-opt
fringe tabulated-list newcomment lisp-mode prog-mode register page
menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core frame cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
case-table epa-hook jka-cmpr-hook help simple abbrev minibuffer nadvice
loaddefs button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote make-network-process cocoa ns
multi-tty emacs)

[Message part 3 (message/rfc822, inline)]

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Jan Djärv <jan.h.d <at> me.com>
Cc: 15802-done <at> debbugs.gnu.org
Subject: Re: bug#15802: 24.3.50; PUSH_HANDLER leaks memory?
Date: Tue, 05 Nov 2013 11:28:00 -0500

> The NULL case must be fairly common, i.e just one PUSH and then unwind.

I'm not sure it's that common, since command_loop pushes a `top-level'
catcher, so as long as we stay within command_loop_2 it doesn't go back
to NULL.

This said we should eliminate the NULL case by putting
a "sentinel" at the beginning, so as to avoid this problem.
I've done that with the patch below,


        Stefan


=== modified file 'src/ChangeLog'
--- src/ChangeLog	2013-11-05 09:00:52 +0000
+++ src/ChangeLog	2013-11-05 16:26:47 +0000
@@ -1,3 +1,12 @@
+2013-11-05  Stefan Monnier  <monnier <at> iro.umontreal.ca>
+
+	* eval.c (handlerlist_sentinel): New variable (bug#15802).
+	(init_eval): Use it to ensure handlerlist is non-NULL.
+	(unwind_to_catch): Make sure we never set handlerlist to NULL.
+	(Fsignal): Adjust NULLness test of handlerlist.
+
+	* lisp.h (PUSH_HANDLER): Assume handlerlist is non-NULL.
+
 2013-11-05  Xue Fuqiao  <xfq.free <at> gmail.com>
 
 	* xdisp.c (syms_of_xdisp): Mention the active display table in doc

=== modified file 'src/eval.c'
--- src/eval.c	2013-10-29 14:46:23 +0000
+++ src/eval.c	2013-11-05 16:24:40 +0000
@@ -237,11 +237,22 @@
   Vrun_hooks = Qnil;
 }
 
+static struct handler handlerlist_sentinel;
+
 void
 init_eval (void)
 {
   specpdl_ptr = specpdl;
-  handlerlist = NULL;
+  { /* Put a dummy catcher at top-level so that handlerlist is never NULL.
+       This is important since handlerlist->nextfree holds the freelist
+       which would otherwise leak every time we unwind back to top-level.   */
+    struct handler *c;
+    handlerlist = handlerlist_sentinel.nextfree = &handlerlist_sentinel;
+    PUSH_HANDLER (c, Qunbound, CATCHER);
+    eassert (c == &handlerlist_sentinel);
+    handlerlist_sentinel.nextfree = NULL;
+    handlerlist_sentinel.next = NULL;
+  }
   Vquit_flag = Qnil;
   debug_on_next_call = 0;
   lisp_eval_depth = 0;
@@ -1129,6 +1140,8 @@
 {
   bool last_time;
 
+  eassert (catch->next);
+
   /* Save the value in the tag.  */
   catch->val = value;
 
@@ -1542,7 +1555,10 @@
     }
   else
     {
-      if (handlerlist != 0)
+      if (handlerlist != &handlerlist_sentinel)
+	/* FIXME: This will come right back here if there's no `top-level'
+	   catcher.  A better solution would be to abort here, and instead
+	   add a catch-all condition handler so we never come here.  */
 	Fthrow (Qtop_level, Qt);
     }
 

=== modified file 'src/lisp.h'
--- src/lisp.h	2013-11-05 07:11:24 +0000
+++ src/lisp.h	2013-11-05 15:48:45 +0000
@@ -2873,13 +2873,12 @@
 
 /* Fill in the components of c, and put it on the list.  */
 #define PUSH_HANDLER(c, tag_ch_val, handlertype)	\
-  if (handlerlist && handlerlist->nextfree)		\
+  if (handlerlist->nextfree)				\
     (c) = handlerlist->nextfree;			\
   else							\
     {							\
       (c) = xmalloc (sizeof (struct handler));		\
       (c)->nextfree = NULL;				\
-      if (handlerlist)					\
 	handlerlist->nextfree = (c);			\
     }							\
   (c)->type = (handlertype);				\

Previous Next

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