nhyatt/putty-source
1
0
Fork 0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-10 01:48:00 +00:00
Code Issues Projects Releases Wiki Activity

Fix winhandl.c's failure to ever free a foreign handle.

Browse Source 
Handles managed by winhandl.c have a 'busy' flag, which is used to
mean two things: (a) is a subthread currently blocked on this handle
so various operations in the main thread have to be deferred until it
finishes? And (b) is this handle currently one that should be returned
to the main loop to be waited for?

For HT_INPUT and HT_OUTPUT, those things are either both true or both
false, so a single flag covering both of them is fine. But HT_FOREIGN
handles have the property that they should always be waited for in the
main loop, but no subthread is blocked on them. The latter means that
operations done on them in the main thread should not be deferred; the
only such operation is cleaning them up in handle_free().

handle_free() was failing to spot this, and was deferring freeing
HT_FOREIGN handles until their subthread terminated - which of course
never happened. As a result, when a named pipe server was closed, its
actual Windows event object got destroyed, but winhandl.c still kept
passing it back to the main thread, leading to a tight loop because
MsgWaitForMultipleObjects would return ERROR_INVALID_HANDLE and never
block.

(cherry picked from commit 431f8db862)
This commit is contained in:
Simon Tatham 2015-09-25 16:03:47 +01:00 committed by Jacob Nevins
parent 72b659cb72
commit 98c946966b
1 changed files with 11 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
windows/winhandl.c
View File

@ -579,17 +579,18 @@ static void handle_destroy(struct handle *h)
void handle_free(struct handle *h)
{
assert(h && !h->u.g.moribund);
if (h->u.g.busy && h->type != HT_FOREIGN) {
/*
* If the handle is currently busy, we cannot immediately free
* it. Instead we must wait until it's finished its current
* it, because its subthread is in the middle of something.
* (Exception: foreign handles don't have a subthread.)
*
* Instead we must wait until it's finished its current
* operation, because otherwise the subthread will write to
* invalid memory after we free its context from under it.
*/
assert(h && !h->u.g.moribund);
if (h->u.g.busy) {
/*
* Just set the moribund flag, which will be noticed next
* time an operation completes.
* invalid memory after we free its context from under it. So
* we set the moribund flag, which will be noticed next time
* an operation completes.
*/
h->u.g.moribund = TRUE;
} else if (h->u.g.defunct) {