From 98c946966b191031f4692a09147bc71d415e44c8 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Fri, 25 Sep 2015 16:03:47 +0100 Subject: [PATCH] Fix winhandl.c's failure to ever free a foreign handle. 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 431f8db86278836adbe63dba7d1ab25fb94b616d) --- windows/winhandl.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/windows/winhandl.c b/windows/winhandl.c index 7c282fc7..b4d508b0 100644 --- a/windows/winhandl.c +++ b/windows/winhandl.c @@ -579,17 +579,18 @@ static void handle_destroy(struct handle *h) void handle_free(struct handle *h) { - /* - * If the handle is currently busy, we cannot immediately free - * it. 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. + if (h->u.g.busy && h->type != HT_FOREIGN) { + /* + * If the handle is currently busy, we cannot immediately free + * 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. 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) { -- 2.45.2