From b3f0fb6baade0d496286973ffef8907bccf04193 Mon Sep 17 00:00:00 2001 From: Corinna Vinschen Date: Thu, 6 Jun 2013 15:29:41 +0000 Subject: [PATCH] * exceptions.cc (_cygtls::handle_SIGCONT): Simplify loop waiting for sig_handle_tty_stop to wake up. Make sure to unlock before calling yield to avoid starvation of sig_handle_tty_stop. Add comments. * miscfuncs.cc (yield): Explain why yield should never be called under _cygtls::lock conditions. Call SleepEx with 1ms timeout. Explain why. --- winsup/cygwin/ChangeLog | 8 ++++++++ winsup/cygwin/exceptions.cc | 27 ++++++++++----------------- winsup/cygwin/miscfuncs.cc | 18 ++++++++++++------ 3 files changed, 30 insertions(+), 23 deletions(-) diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog index f307ee691..956d10087 100644 --- a/winsup/cygwin/ChangeLog +++ b/winsup/cygwin/ChangeLog @@ -1,3 +1,11 @@ +2013-06-06 Corinna Vinschen + + * exceptions.cc (_cygtls::handle_SIGCONT): Simplify loop waiting for + sig_handle_tty_stop to wake up. Make sure to unlock before calling + yield to avoid starvation of sig_handle_tty_stop. Add comments. + * miscfuncs.cc (yield): Explain why yield should never be called under + _cygtls::lock conditions. Call SleepEx with 1ms timeout. Explain why. + 2013-06-05 Corinna Vinschen * include/cygwin/version.h (CYGWIN_VERSION_DLL_MINOR): Bump to 20. diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc index abb973528..aa0036a18 100644 --- a/winsup/cygwin/exceptions.cc +++ b/winsup/cygwin/exceptions.cc @@ -1253,25 +1253,18 @@ _cygtls::handle_SIGCONT () { myself->stopsig = 0; myself->process_state &= ~PID_STOPPED; - int state = 0; /* Carefully tell sig_handle_tty_stop to wake up. */ - while (state < 2) - { - lock (); - if (sig) - yield (); /* state <= 1 */ - else if (state) - state++; /* state == 2 */ - else - { - sig = SIGCONT; - SetEvent (signal_arrived); - state++; /* state == 1 */ - } - unlock (); - } + lock (); + sig = SIGCONT; + SetEvent (signal_arrived); + /* Make sure yield doesn't run under lock condition to avoid + starvation of sig_handle_tty_stop. */ + unlock (); + /* Wait until sig_handle_tty_stop woke up. */ + while (sig) + yield (); /* Tell wait_sig to handle any queued signals now that we're alive - again. */ + again. */ sig_dispatch_pending (false); } /* Clear pending stop signals */ diff --git a/winsup/cygwin/miscfuncs.cc b/winsup/cygwin/miscfuncs.cc index 6a204a01e..d0748ea6c 100644 --- a/winsup/cygwin/miscfuncs.cc +++ b/winsup/cygwin/miscfuncs.cc @@ -236,7 +236,11 @@ check_iovec (const struct iovec *iov, int iovcnt, bool forwrite) return (ssize_t) tot; } -/* Try hard to schedule another thread. */ +/* Try hard to schedule another thread. + + Note: Don't call yield under _cygtls::lock conditions. It results in + potential starvation, especially on a single-CPU system, because + _cygtls::lock also calls yield when waiting for the lock. */ void yield () { @@ -244,11 +248,13 @@ yield () SetThreadPriority (GetCurrentThread (), THREAD_PRIORITY_IDLE); for (int i = 0; i < 2; i++) { - /* MSDN implies that SleepEx(0,...) will force scheduling of other - threads. Unlike SwitchToThread() the documentation does not mention - other cpus so, presumably (hah!), this + using a lower priority will - stall this thread temporarily and cause another to run. */ - SleepEx (0, false); + /* MSDN implies that SleepEx will force scheduling of other threads. + Unlike SwitchToThread() the documentation does not mention other + cpus so, presumably (hah!), this + using a lower priority will + stall this thread temporarily and cause another to run. + Note: Don't use 0 timeout. This takes a lot of CPU if something + goes wrong. */ + SleepEx (1L, false); } SetThreadPriority (GetCurrentThread (), prio); }