--- Begin Message ---
|
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 address@hidden
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)
--- End Message ---
--- Begin Message ---
|
Subject: |
Re: bug#15802: 24.3.50; PUSH_HANDLER leaks memory? |
|
Date: |
Tue, 05 Nov 2013 11:28:00 -0500 |
|
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3.50 (gnu/linux) |
> 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 <address@hidden>
+
+ * 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 <address@hidden>
* 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); \
--- End Message ---