From: Stefan Monnier <monnier@iro.umontreal.ca>
To: "Jan Djärv" <jan.h.d@me.com>
Cc: 15802-done@debbugs.gnu.org
Subject: bug#15802: 24.3.50; PUSH_HANDLER leaks memory?
Date: Tue, 05 Nov 2013 11:28:00 -0500 [thread overview]
Message-ID: <jwv1u2uc0of.fsf-monnier+emacsbugs@gnu.org> (raw)
In-Reply-To: <02F4E04B-C60F-481D-B381-79F1F8867D59@me.com>
> 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@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@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); \
prev parent reply other threads:[~2013-11-05 16:28 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-04 18:32 bug#15802: 24.3.50; PUSH_HANDLER leaks memory? Jan Djärv
2013-11-04 20:10 ` Stefan Monnier
2013-11-04 21:49 ` Jan Djärv
2013-11-05 2:48 ` Stefan Monnier
2013-11-05 6:53 ` Jan Djärv
2013-11-05 16:28 ` Stefan Monnier [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=jwv1u2uc0of.fsf-monnier+emacsbugs@gnu.org \
--to=monnier@iro.umontreal.ca \
--cc=15802-done@debbugs.gnu.org \
--cc=jan.h.d@me.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).