From: Simon Tatham <anakin@pobox.com>
Date: Sat, 7 Feb 2015 11:48:49 +0000 (+0000)
Subject: Improve comments in winhandl.c.
X-Git-Tag: 0.68~632
X-Git-Url: https://asedeno.scripts.mit.edu/gitweb/?a=commitdiff_plain;ds=sidebyside;h=a87a14ae0fc7d4621b5b1fafdb2053b3638b4b2b;hp=-c;p=PuTTY.git

Improve comments in winhandl.c.

To understand the handle leak bug that I fixed in git commit
7549f2da40d3666f2c9527d84d9ed5468e231691, I had to think fairly hard
to remind myself what all this code was doing, which means the
comments weren't good enough. Expanded and rewritten some of them in
the hope that things will be clearer next time.
---

a87a14ae0fc7d4621b5b1fafdb2053b3638b4b2b
diff --git a/windows/winhandl.c b/windows/winhandl.c
index 6b129ad8..d81054f3 100644
--- a/windows/winhandl.c
+++ b/windows/winhandl.c
@@ -167,13 +167,27 @@ static DWORD WINAPI handle_input_threadfunc(void *param)
 
 	SetEvent(ctx->ev_to_main);
 
-	if (!ctx->len)
+	if (!ctx->len) {
+            /*
+             * 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, so we must not touch ctx at all
+             * after this.
+             */
 	    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 +294,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 +324,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)
@@ -601,10 +629,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);