+2012-12-28 Christopher Faylor <me.cygwin2012@cgf.cx>
+
+ * DevNotes: Add entry cgf-000019.
+ * dcrt0.cc (do_exit): Just set exit_state to ES_EVENTS_TERMINATE and
+ nuke call to events_terminate which just set a superfluous flag.
+ * sigproc.cc (signal_exit_code): New variable.
+ (setup_signal_exit): Define new function.
+ (_cygtls::signal_exit): Remove accommodations for closing the signal
+ pipe handle.
+ (exit_thread): Just sleep if we're exiting.
+ (wait_sig): If signal_exit_code is set, just handle bookkeeping signals
+ and exit ReadFile loop if there is nothing more to process. Call
+ signal_exit at end if signal_exit_code is non-zero.
+ * sigproc.h (setup_signal_exit): Declare new function.
+ * exceptions.cc (sigpacket::process): Use setup_signal_exit to control
+ exiting due to a signal.
+ (exception::handle): Ditto. Query exit_state rather than defunct
+ exit_already to determine if we are exiting.
+ * globals.cc (ES_SIGNAL_EXIT): New enum.
+ * sync.h (lock_process::release): New function for explicitly unlocking
+ muto.
+ (lock_process::~lock_process): Use release method.
+
2012-12-27 Christopher Faylor <me.cygwin2012@cgf.cx>
* fork.cc (child_info::prefork): Fix error message formatting.
+2012-12-28 cgf-000019
+
+(I forgot to mention that cgf-000018 was reverted. Although I never saw
+a hang from this, I couldn't convince myself that one wasn't possible.)
+
+This fix attempts to correct a deadlock where, when a true Windows
+signal arrives, Windows creates a thread which "does stuff" and attempts
+to exit. In the process of exiting Cygwin grabs the process lock. If
+the signal thread has seen the signal and wants to exit, it can't
+because the newly-created thread now holds it. But, since the new
+thread is relying on the signal thread to release its process lock,
+it exits and the process lock is never released.
+
+To fix this, I removed calls to _cygtls::signal_exit in favor of
+flagging that we were exiting by setting signal_exit_code (almost forgot
+to mark that NO_COPY: that would have been fun). The new function
+setup_signal_exit() now handles setting things up so that ReadFile loop
+in wait_sig will do the right thing when it terminates. This function
+may just Sleep indefinitely if a signal is being sent from a thread
+other than the signal thread. wait_sig() was changed so that it will
+essentially drop into asychronous-read-mode when a signal which exits
+has been detected. The ReadFile loop is exited when we know that the
+process is supposed to be exiting and there is nothing else in the
+signal queue.
+
+Although I never actually saw this happen, exit_thread() was also
+changed to release the process lock and just sleep indefintely if it is
+detected that we are exiting.
+
2012-12-21 cgf-000018
Re: cgf-000017
/* This is set to indicate that we have already exited. */
-static NO_COPY int exit_already = 0;
-
NO_COPY static struct
{
unsigned int code;
return 0;
}
- /* If we've already exited, don't do anything here. Returning 1
+ /* If we're exiting, don't do anything here. Returning 1
tells Windows to keep looking for an exception handler. */
- if (exit_already || e->ExceptionFlags)
+ if (exit_state || e->ExceptionFlags)
return 1;
siginfo_t si = {0};
error_code);
}
- /* Flag signal + core dump */
- me.signal_exit ((cygheap->rlim_core > 0UL ? 0x80 : 0) | si.si_signo);
+ setup_signal_exit ((cygheap->rlim_core > 0UL ? 0x80 : 0) | si.si_signo);
}
si.si_addr = (si.si_signo == SIGSEGV || si.si_signo == SIGBUS
return rc;
exit_sig:
- tls->signal_exit (si.si_signo); /* never returns */
-}
-
-void
-events_terminate ()
-{
- exit_already = 1;
+ sigproc_printf ("setting up for exit with signal %d", si.si_signo);
+ setup_signal_exit (si.si_signo);
+ return rc;
}
int
Static HANDLE my_sendsig;
Static HANDLE my_readsig;
+Static int signal_exit_code;
/* Function declarations */
static int __stdcall checkstate (waitq *) __attribute__ ((regparm (1)));
ForceCloseHandle1 (h, my_readsig);
}
-/* Cover function to `do_exit' to handle exiting even in presence of more
- exceptions. We used to call exit, but a SIGSEGV shouldn't cause atexit
- routines to run. */
+/* Exit due to a signal, even in presence of more exceptions. We used to just
+ call exit, but a SIGSEGV shouldn't cause atexit routines to run.
+ Should only be called from the signal thread. */
void
_cygtls::signal_exit (int rc)
{
- HANDLE myss = my_sendsig;
- my_sendsig = NULL; /* Make no_signals_allowed return true */
-
- /* This code used to try to always close my_readsig but it ended up
- blocking for reasons that people in google think make sense.
- It's possible that it was blocking because ReadFile was still active
- but it isn't clear why this only caused random hangs rather than
- consistent hangs. So, for now at least, avoid closing my_readsig
- unless this is the signal thread. */
- if (&_my_tls == _sig_tls)
- close_my_readsig (); /* Stop any currently executing sig_sends */
- else
- {
- sigpacket sp = {};
- sp.si.si_signo = __SIGEXIT;
- DWORD len;
- /* Write a packet to the wait_sig thread which tells it to exit and
- close my_readsig. */
- WriteFile (myss, &sp, sizeof (sp), &len, NULL);
- }
signal_debugger (rc & 0x7f);
if (rc == SIGQUIT || rc == SIGABRT)
}
}
+/* Set up stuff so that the signal thread will know that we are
+ exiting due to a signal. */
+void
+setup_signal_exit (int sig)
+{
+ signal_exit_code = sig; /* Tell wait_sig() that we are exiting. */
+ exit_state = ES_SIGNAL_EXIT; /* Tell the rest of the world that we are exiting. */
+
+ if (&_my_tls != _sig_tls)
+ {
+ sigpacket sp = {};
+ sp.si.si_signo = __SIGEXIT;
+ DWORD len;
+ /* Write a packet to the wait_sig thread. It will eventuall cause
+ the process to exit too. So just wait for that to happen after
+ sending the packet. */
+ WriteFile (my_sendsig, &sp, sizeof (sp), &len, NULL);
+ Sleep (INFINITE);
+ }
+}
+
/* Exit the current thread very carefully.
See cgf-000017 in DevNotes for more details on why this is
necessary. */
siginfo_t si = {__SIGTHREADEXIT, SI_KERNEL};
si.si_value.sival_ptr = h;
lock_process for_now; /* May block indefinitely if we're exiting. */
+ if (exit_state)
+ {
+ for_now.release ();
+ Sleep (INFINITE);
+ }
+
/* Tell wait_sig to wait for this thread to exit. It can then release
the lock below and close the above-opened handle. */
sig_send (myself_nowait, si, &_my_tls);
- ExitThread (0); /* Should never hit this */
+ ExitThread (0);
}
int __stdcall
static void WINAPI
wait_sig (VOID *)
{
+ extern int signal_exit_code;
_sig_tls = &_my_tls;
sig_hold = CreateEvent (&sec_none_nih, FALSE, FALSE, NULL);
{
if (pack.si.si_signo == __SIGHOLD)
WaitForSingleObject (sig_hold, INFINITE);
+
DWORD nb;
+ /* If signal_exit_code is set then we are shutting down due to a signal.
+ We'll exit this loop iff there is nothing more in the signal queue. */
+ if (signal_exit_code
+ && (!PeekNamedPipe (my_readsig, NULL, 0, NULL, &nb, NULL) || !nb))
+ break;
+
pack.sigtls = NULL;
if (!ReadFile (my_readsig, &pack, sizeof (pack), &nb, NULL))
break;
continue;
}
+ if (signal_exit_code && pack.si.si_signo > 0)
+ continue; /* No more real signals allowed */
+
sigset_t dummy_mask;
if (!pack.mask)
{
break;
}
- close_my_readsig ();
sigproc_printf ("signal thread exiting");
+
+ my_sendsig = NULL; /* Make no_signals_allowed return true */
+ close_my_readsig (); /* Cause any sig_send's to stop */
+
+ if (signal_exit_code)
+ _my_tls.signal_exit (signal_exit_code);
+
/* Just wait for the process to go away. Otherwise, this thread's
exit value could be interpreted as the process exit value.
See cgf-000017 in DevNotes for more details. */
git://sourceware.org / newlib-cygwin.git / commitdiff