* Coredump on C-c - followup
@ 2013-11-17 23:49 Eitan Adler
0 siblings, 0 replies; only message in thread
From: Eitan Adler @ 2013-11-17 23:49 UTC (permalink / raw)
To: Zsh hackers list, Bart Schaefer
Hey,
I'm on the list so I missed your last reply to me. Please keep me on
the CC list ;)
On Wed, Oct 16, 2013 at 2:40 PM, Eitan Adler <lists@xxxxxxxxxxxxxx> wrote:
>
> I'm hoping to prod about this again, this time with a slightly
> different backtrace.
> "Slightly" is a bit of an understatement; that's an almost entirely
> different backtrace.
Heh.
> I'm also scratching my head a bit about why you're able to land
> interrupts in inconvenient locations so easily when nobody else has
> reported this kind of issue in several years.
> What follows is somewhat stream-of-consciousness, sorry.
> In this new case you appear to have a trap set on the INT signal
Yes. The trap is available here:
https://code.google.com/p/config/source/browse/zsh/precmd.zsh#143
My entire config environment is available in that repo. I did not
mention it in my original emails since I completely forgot about it.
> which was invoked because you interrupted your setCurrentPS1
> precmd-hook, and then had a CHLD signal come in while the INT trap was
> unwinding itself.
> All of which appears to have been possible because __vcs_dir was
> taking a long time?
My disk is quite slow, so I would not be surprised.
> Signals are being blocked or queued in most of the appropriate places
> -- the zhandler at the top of the backtrace is being run when zfree is
> already finished and releases the signal queue, and the zhandler in
> the middle of the trace is being invoked while signals are unblocked
> to wait for _vcs_dir to exit.
> At first I thought execrestore() needs to queue signals just as
> endparamscope() does. Why you (Eitan) seem to be uniquely able to
> deliver signals into the middle of functions that are little more than
> a few assignment statements must have something to do with your
> hardware environment. I guess both of those functions are a bit more
> than that because they free memory in addition to moving pointers
> around.
>
> On closer examination I'm not sure that will work, because all of this
> is happening inside waitforpid() which prefaces it with an explicit
> dont_queue_signals(). So it's not impossible that we're also open to
> improper signaling inside zfree() here, but the stack trace makes that
> look unlikely. Still, it seems to be possible to reorder operations
> in execrestore() to avoid the immediate problem of exstack pointing to
> the wrong thing. That's at least worthwhile, though the possibility
> of an execsave/execrestore pair occurring in the middle of an
> execrestore in progress makes my head hurt, so I think perhaps both
> changes are in order.
Ha!
> execsave(), incidentally, already does things in a "safe" order, so it
> probably doesn't need the signal queuing.
The patch below seems mangled. Maybe this was the mail archive I
used. Can you resend as a link or patch?
diff --git a/Src/exec.c b/Src/exec.c
index de1b484..8efbbd4 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -5078,29 +5078,33 @@ execsave(void)
void
execrestore(void)
{
- struct execstack *en;
+ struct execstack *en = exstack;
DPUTS(!exstack, "BUG: execrestore() without execsave()");
- list_pipe_pid = exstack->list_pipe_pid;
- nowait = exstack->nowait;
- pline_level = exstack->pline_level;
- list_pipe_child = exstack->list_pipe_child;
- list_pipe_job = exstack->list_pipe_job;
- strcpy(list_pipe_text, exstack->list_pipe_text);
- lastval = exstack->lastval;
- noeval = exstack->noeval;
- badcshglob = exstack->badcshglob;
- cmdoutpid = exstack->cmdoutpid;
- cmdoutval = exstack->cmdoutval;
- use_cmdoutval = exstack->use_cmdoutval;
- trap_return = exstack->trap_return;
- trap_state = exstack->trap_state;
- trapisfunc = exstack->trapisfunc;
- traplocallevel = exstack->traplocallevel;
- noerrs = exstack->noerrs;
- setunderscore(exstack->underscore);
- zsfree(exstack->underscore);
- en = exstack->next;
- free(exstack);
- exstack = en;
+
+ queue_signals();
+ exstack = exstack->next;
+
+ list_pipe_pid = en->list_pipe_pid;
+ nowait = en->nowait;
+ pline_level = en->pline_level;
+ list_pipe_child = en->list_pipe_child;
+ list_pipe_job = en->list_pipe_job;
+ strcpy(list_pipe_text, en->list_pipe_text);
+ lastval = en->lastval;
+ noeval = en->noeval;
+ badcshglob = en->badcshglob;
+ cmdoutpid = en->cmdoutpid;
+ cmdoutval = en->cmdoutval;
+ use_cmdoutval = en->use_cmdoutval;
+ trap_return = en->trap_return;
+ trap_state = en->trap_state;
+ trapisfunc = en->trapisfunc;
+ traplocallevel = en->traplocallevel;
+ noerrs = en->noerrs;
+ setunderscore(en->underscore);
+ zsfree(en->underscore);
+ free(en);
+
+ unqueue_signals();
}
--
Eitan Adler
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2013-11-17 23:50 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-17 23:49 Coredump on C-c - followup Eitan Adler
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/zsh/
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 NNTP newsgroup(s).