diff options
author | Christopher Faylor <me@cgf.cx> | 2005-02-11 18:24:15 +0300 |
---|---|---|
committer | Christopher Faylor <me@cgf.cx> | 2005-02-11 18:24:15 +0300 |
commit | cc9440b6f42536f9f3b3fca6a6c53155792a51eb (patch) | |
tree | 7839100d471d0478d72881287053dde865e70bb4 /winsup/cygwin/cygthread.cc | |
parent | 199bf79367fd838d12900e70c05756f2bc29db5d (diff) |
* cygthread.cc (cygthread::release): Reset ev here if it exists.
(cygthread::terminate_thread): Eliminat racy code which reset ev and
thread_sync. Remove a few nonsensical inuse checks. Exit at the bottom.
(cygthread::detach): Rewrite to again try to ensure that we don't say we're
signalled when we are not signalled.
* fhandler.cc (fhandler_base::raw_read): Revert to signalling read success
quickly.
* pipe.cc (fhandler_pipe::close): Use base method to close handle.
* sigproc.h (WAIT_SIG_PRIORITY): Just trundle along at normal priority to allow
the pipe thread to do its thing if possible.
* pinfo.h (pinfo::zap_cwd): Declare new function.
(pinfo::zap_cwd): Move 'cd out of the way code' here.
(pinfo::exit): Use it here.
* spawn.cc (spawn_guts): And here.
Diffstat (limited to 'winsup/cygwin/cygthread.cc')
-rw-r--r-- | winsup/cygwin/cygthread.cc | 169 |
1 files changed, 127 insertions, 42 deletions
diff --git a/winsup/cygwin/cygthread.cc b/winsup/cygwin/cygthread.cc index 4c14f1954..a5e52be00 100644 --- a/winsup/cygwin/cygthread.cc +++ b/winsup/cygwin/cygthread.cc @@ -234,6 +234,8 @@ cygthread::release (bool nuke_h) #endif __name = NULL; func = NULL; + if (ev) + ResetEvent (ev); if (!InterlockedExchange (&inuse, 0)) #ifdef DEBUGGING api_fatal ("released a thread that was not inuse"); @@ -247,37 +249,22 @@ bool cygthread::terminate_thread () { bool terminated = true; - /* FIXME: The if (!inuse) stuff below should be handled better. The - problem is that terminate_thread could be called while a thread - is terminating and either the thread could be handling its own - release or, if this is being called during exit, some other - thread may be attempting to free up this resource. In the former - case, setting some kind of "I deal with my own exit" type of - flag may be the way to handle this. */ - if (!is_freerange) - { - ResetEvent (*this); - ResetEvent (thread_sync); - } - debug_printf ("thread '%s', id %p, inuse %d, stack_ptr %p", name (), id, inuse, stack_ptr); while (inuse && !stack_ptr) low_priority_sleep (0); if (!inuse) - return false; - + goto force_notterminated; + (void) TerminateThread (h, 0); (void) WaitForSingleObject (h, INFINITE); - if (ev) - terminated = WaitForSingleObject (ev, 0) != WAIT_OBJECT_0; - if (!inuse || exiting) - return false; - CloseHandle (h); - if (!inuse) - return false; + if (!inuse || exiting) + goto force_notterminated; + + if (ev) + terminated = WaitForSingleObject (ev, 0) != WAIT_OBJECT_0; MEMORY_BASIC_INFORMATION m; memset (&m, 0, sizeof (m)); @@ -289,8 +276,6 @@ cygthread::terminate_thread () debug_printf ("VirtualFree of allocation base %p<%p> failed, %E", stack_ptr, m.AllocationBase); - if (!inuse) - /* nothing */; if (is_freerange) free (this); else @@ -300,6 +285,12 @@ cygthread::terminate_thread () #endif release (true); } + + goto out; + +force_notterminated: + terminated = false; +out: return terminated; } @@ -311,7 +302,7 @@ bool cygthread::detach (HANDLE sigwait) { bool signalled = false; - bool terminated = false; + bool thread_was_reset = false; if (!inuse) system_printf ("called detach but inuse %d, thread %p?", inuse, id); else @@ -322,35 +313,61 @@ cygthread::detach (HANDLE sigwait) res = WaitForSingleObject (*this, INFINITE); else { + /* Lower our priority and give priority to the read thread */ + HANDLE hth = GetCurrentThread (); + LONG prio = GetThreadPriority (hth); + (void) ::SetThreadPriority (hth, THREAD_PRIORITY_IDLE); + HANDLE w4[2]; - w4[0] = *this; + unsigned n = 2; + DWORD howlong = INFINITE; + w4[0] = sigwait; w4[1] = signal_arrived; - res = WaitForSingleObject (sigwait, INFINITE); - if (res != WAIT_OBJECT_0) - system_printf ("WFSO sigwait %p failed, res %u, %E", sigwait, res); - res = WaitForMultipleObjects (2, w4, FALSE, INFINITE); + /* For a description of the below loop see the end of this file */ + for (int i = 0; i < 2; i++) + switch (res = WaitForMultipleObjects (n, w4, FALSE, howlong)) + { + case WAIT_OBJECT_0: + if (n == 1) + howlong = 50; + break; + case WAIT_OBJECT_0 + 1: + n = 1; + if (i--) + howlong = 50; + break; + case WAIT_TIMEOUT: + break; + default: + if (!exiting) + api_fatal ("WFMO failed waiting for cygthread '%s'", __name); + break; + } + /* WAIT_OBJECT_0 means that the thread successfully read something, + so wait for the cygthread to "terminate". */ if (res == WAIT_OBJECT_0) - signalled = false; - else if (res != WAIT_OBJECT_0 + 1) - api_fatal ("WFMO failed waiting for cygthread '%s'", __name); - else if ((res = WaitForSingleObject (*this, 0)) == WAIT_OBJECT_0) - signalled = false; + (void) WaitForSingleObject (*this, INFINITE); else { - terminated = true; + /* Thread didn't terminate on its own, so maybe we have to + do it. */ signalled = terminate_thread (); + /* Possibly the thread completed *just* before it was + terminated. Detect this. If this happened then the + read was not terminated on a signal. */ + if (WaitForSingleObject (sigwait, 0) == WAIT_OBJECT_0) + signalled = false; + if (signalled) + set_sig_errno (EINTR); + thread_was_reset = true; } - if (WaitForSingleObject (sigwait, 0) == WAIT_OBJECT_0) - signalled = false; - else if (signalled) - set_sig_errno (EINTR); /* caller should be dealing with return - values. */ + (void) ::SetThreadPriority (hth, prio); } thread_printf ("%s returns %d, id %p", sigwait ? "WFMO" : "WFSO", res, id); - if (terminated) + if (thread_was_reset) /* already handled */; else if (is_freerange) { @@ -372,3 +389,71 @@ cygthread::terminate () { exiting = 1; } + +/* The below is an explanation of synchronization loop in cygthread::detach. + The intent is that the loop will always try hard to wait for both + synchronization events from the reader thread but will exit with + res == WAIT_TIMEOUT if a signal occurred and the reader thread is + still blocked. + + case 0 - no signal + + i == 0 (howlong == INFINITE) + W0 activated + howlong not set because n != 1 + just loop + + i == 1 (howlong == INFINITE) + W0 activated + howlong not set because n != 1 + just loop (to exit loop) - no signal + + i == 2 (howlong == INFINITE) + exit loop + + case 1 - signal before thread initialized + + i == 0 (howlong == INFINITE) + WO + 1 activated + n set to 1 + howlong untouched because i-- == 0 + loop + + i == 0 (howlong == INFINITE) + W0 must be activated + howlong set to 50 because n == 1 + + i == 1 (howlong == 50) + W0 activated + loop (to exit loop) - no signal + + WAIT_TIMEOUT activated + signal potentially detected + loop (to exit loop) + + i == 2 (howlong == 50) + exit loop + + case 2 - signal after thread initialized + + i == 0 (howlong == INFINITE) + W0 activated + howlong not set because n != 1 + loop + + i == 1 (howlong == INFINITE) + W0 + 1 activated + n set to 1 + howlong set to 50 because i-- != 0 + loop + + i == 1 (howlong == 50) + W0 activated + loop (to exit loop) - no signal + + WAIT_TIMEOUT activated + loop (to exit loop) - signal + + i == 2 (howlong == 50) + exit loop +*/ |