Re: Deadlock when receiving kill-signal from child process

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Aug 7,  3:45am, Mathias Fredriksson wrote:
}
} Sadly I can't be of much assistance here, I believe you can't call
} pthread mutexes from a signal handler

If true, that would mean either (a) you can't use stdio functions from
a signal handler, because this implementation of stdio is using pthread
mutexes, or (b) stdio is not allowed to use pthread mutexes, so this 
implementation is broken.

} but that isn't whats happening
} here? If I understand correctly a signal is received while a mutex
} lock is (being) aquired.

Specifially ferror() is attempting to acquire a mutex lock when the
signal arrives, and then the handler calls fputc() which tries to
acquire the same lock, and: clunk.

} I'm not quite sure I understand what these changes do

queue_signals() increments a counter that is tested in the signal
handler.  If the counter is nonzero, the handler does nothing but
make note in a static array that the signal was received.  If too
many signals arrive before there is an opportunity to empty the
array, the excess are simply dropped.  (The default is to be able
to queue 128 signals before starting to lose some -- if you're
expecting the shell to handle massive numbers of signals arriving
in quick succession, you're using the wrong tool for your job.
Further, this is another reason I'm reluctant to adopt the WINCH
approach for signals in general.)

dont_queue_signals() forces the counter to zero and calls all the
shell function handlers for the signals in the static array, in the
order the signals were recorded.

restore_queue_signals() sets the counter back to some previous state
(obtained from queue_signal_level()).

unqueue_signals() decrements the counter and calls all the shell
functions if (when) it reaches zero.

Using the counter means that you can have arbitrarily deep nesting of
queue_signals().  Using dont_queue_signals() implies that it is known
to be safe to run the handlers at that time, because it unwinds all
those levels of nesting at once.

So anytime I find myself using dont_queue_signals() I worry that there
is a calling scope reason avoid signal handlers.  On the other hand,
if it's safe to run shell code at all (e.g., runshfunc()), it should
also be safe to run signal traps.

} but at least
} this last patch made it a lot harder for me to have zsh lock up. I had
} to leave my script running in a while true; do ...; done loop
} (eventually, 30sec-10min it would hit a lock).
} 
} #0  0x00007fff8abfe72a in __sigsuspend ()
} #1  0x0000000107b59287 in signal_suspend ()
} #2  0x0000000107b30671 in zwaitjob ()

Sigh.  That looks like another case of waiting for a child that does
not exist.  Unfortunately this time the call stack beyond here does
not hint at where that job was started.  Might need another strace
that corresponds to this backtrace.

} This just seems like the same mutex stuff again:
} 
} #0  0x00007fff8abfe166 in __psynch_mutexwait ()
} #1  0x00007fff8e4b578a in _pthread_mutex_lock ()
} #2  0x00007fff82ce5750 in fputc ()
} #3  0x0000000102c20cd5 in zputs ()
} #4  0x0000000102c20b3c in mb_niceformat ()
} #5  0x0000000102c201cd in zwarning ()
} #6  0x0000000102c20376 in zwarn ()
} #7  0x0000000102c143da in wait_for_processes ()
} #8  0x0000000102c140a6 in zhandler ()
} #9  <signal handler called>
} #10 0x00007fff8abfe72a in __sigsuspend ()

The stack must go further than this?  What called __sigsuspend() ?


} This also looks vaguely familiar but might as well post it:
} 
} #0  0x00007fff8abf95da in syscall_thread_switch ()
} #1  0x00007fff853a982d in _OSSpinLockLockSlow ()
} #2  0x00007fff896d771b in szone_malloc_should_clear ()

Yeah, this is a signal arriving during memory allocation.  Looks like
it came from here:

} #14 0x00007fff896d98d6 in szone_free_definite_size ()
} #15 0x0000000101ca5874 in execlist ()

So we definitely need queue_signals() somewhere down in the guts of
execlist().  Which means rejiggering some of the stuff from previous
patches, so I think I'd better back up and send a new patch against
the git master.

What next concerns me is being sure that signals are unqueued often
enough that interrupts still work, etc.


} Bonus NO_TRAPS_ASYNC:
} 
} #0  0x00007fff8abfe72a in __sigsuspend ()
} #1  0x0000000107509287 in signal_suspend ()
} #11 0x00000001075090b0 in zhandler ()
} #12 <signal handler called>
} #13 0x00007fff8abfe97a in write$NOCANCEL ()
} #14 0x00007fff82ceb9ed in _swrite ()
} #15 0x00007fff82ce44a7 in __sflush ()
} #16 0x00007fff82ce43f5 in fflush ()
} #17 0x0000000107515376 in zwarn ()
} #18 0x00000001075093da in wait_for_processes ()
} #19 0x00000001075090a6 in zhandler ()
} #20 <signal handler called>
} #21 0x00007fff8abffa12 in sigprocmask ()

This trace must also go further?  The part shown is calling a handler
while during fflush() in a previous handler, but the trace doesn't
go back far enough to see where the first handler was called.