X-Git-Url: https://asedeno.scripts.mit.edu/gitweb/?a=blobdiff_plain;f=windows%2Fwinhandl.c;h=e5a5e2d541aea5c00848bd8d314fd33a65f50636;hb=1de7240eb88fa24a8532ded116b4ec72dd213008;hp=6b129ad886591c4fb9d5dd73c9935ede24089b9f;hpb=087ca595f3f612f0d2de1d406b5fcc751cdd3566;p=PuTTY.git

diff --git a/windows/winhandl.c b/windows/winhandl.c
index 6b129ad8..e5a5e2d5 100644
--- a/windows/winhandl.c
+++ b/windows/winhandl.c
@@ -115,7 +115,7 @@ static DWORD WINAPI handle_input_threadfunc(void *param)
     struct handle_input *ctx = (struct handle_input *) param;
     OVERLAPPED ovl, *povl;
     HANDLE oev;
-    int readret, readlen;
+    int readret, readlen, finished;
 
     if (ctx->flags & HANDLE_FLAG_OVERLAPPED) {
 	povl = &ovl;
@@ -165,15 +165,31 @@ static DWORD WINAPI handle_input_threadfunc(void *param)
 	    (ctx->flags & HANDLE_FLAG_IGNOREEOF))
 	    continue;
 
+        /*
+         * If we just set ctx->len to 0, that means the read operation
+         * has returned end-of-file. Telling that to the main thread
+         * will cause it to set its 'defunct' flag and dispose of the
+         * handle structure at the next opportunity, in which case we
+         * mustn't touch ctx at all after the SetEvent. (Hence we do
+         * even _this_ check before the SetEvent.)
+         */
+        finished = (ctx->len == 0);
+
 	SetEvent(ctx->ev_to_main);
 
-	if (!ctx->len)
+	if (finished)
 	    break;
 
 	WaitForSingleObject(ctx->ev_from_main, INFINITE);
 	if (ctx->done) {
+            /*
+             * The main thread has asked us to shut down. Send back an
+             * event indicating that we've done so. Hereafter we must
+             * not touch ctx at all, because the main thread might
+             * have freed it.
+             */
             SetEvent(ctx->ev_to_main);
-	    break;		       /* main thread told us to shut down */
+            break;
         }
     }
 
@@ -280,6 +296,12 @@ static DWORD WINAPI handle_output_threadfunc(void *param)
     while (1) {
 	WaitForSingleObject(ctx->ev_from_main, INFINITE);
 	if (ctx->done) {
+            /*
+             * The main thread has asked us to shut down. Send back an
+             * event indicating that we've done so. Hereafter we must
+             * not touch ctx at all, because the main thread might
+             * have freed it.
+             */
 	    SetEvent(ctx->ev_to_main);
 	    break;
 	}
@@ -304,8 +326,16 @@ static DWORD WINAPI handle_output_threadfunc(void *param)
 	}
 
 	SetEvent(ctx->ev_to_main);
-	if (!writeret)
+	if (!writeret) {
+            /*
+             * The write operation has suffered an error. Telling that
+             * to the main thread will cause it to set its 'defunct'
+             * flag and dispose of the handle structure at the next
+             * opportunity, so we must not touch ctx at all after
+             * this.
+             */
 	    break;
+        }
     }
 
     if (povl)
@@ -381,9 +411,9 @@ static int handle_cmp_evtomain(void *av, void *bv)
     struct handle *a = (struct handle *)av;
     struct handle *b = (struct handle *)bv;
 
-    if ((unsigned)a->u.g.ev_to_main < (unsigned)b->u.g.ev_to_main)
+    if ((uintptr_t)a->u.g.ev_to_main < (uintptr_t)b->u.g.ev_to_main)
 	return -1;
-    else if ((unsigned)a->u.g.ev_to_main > (unsigned)b->u.g.ev_to_main)
+    else if ((uintptr_t)a->u.g.ev_to_main > (uintptr_t)b->u.g.ev_to_main)
 	return +1;
     else
 	return 0;
@@ -394,9 +424,9 @@ static int handle_find_evtomain(void *av, void *bv)
     HANDLE *a = (HANDLE *)av;
     struct handle *b = (struct handle *)bv;
 
-    if ((unsigned)*a < (unsigned)b->u.g.ev_to_main)
+    if ((uintptr_t)*a < (uintptr_t)b->u.g.ev_to_main)
 	return -1;
-    else if ((unsigned)*a > (unsigned)b->u.g.ev_to_main)
+    else if ((uintptr_t)*a > (uintptr_t)b->u.g.ev_to_main)
 	return +1;
     else
 	return 0;
@@ -549,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) {
@@ -601,10 +632,12 @@ void handle_got_event(HANDLE event)
 
     if (h->u.g.moribund) {
 	/*
-	 * A moribund handle is already treated as dead from the
-	 * external user's point of view, so do nothing with the
-	 * actual event. Just signal the thread to die if
-	 * necessary, or destroy the handle if not.
+	 * A moribund handle is one which we have either already
+	 * signalled to die, or are waiting until its current I/O op
+	 * completes to do so. Either way, it's treated as already
+	 * dead from the external user's point of view, so we ignore
+	 * the actual I/O result. We just signal the thread to die if
+	 * we haven't yet done so, or destroy the handle if not.
 	 */
 	if (h->u.g.done) {
 	    handle_destroy(h);