Refactor request cleanup to be handled entirely by worker thread

Author: yungcomputerchair

ffrunner.h

@@ -126,7 +126,7 @@ extern UINT ioMsg;
 void register_get_request(const char *url, bool doNotify, void *notifyData);
 void register_post_request(const char *url, bool doNotify, void *notifyData, uint32_t postDataLen, const char *postData);
 void register_temp_request(const char *url, HANDLE outFile, HANDLE onDone);
-bool handle_io_progress(Request *req);
+void handle_io_progress(Request *req);
 void submit_request(Request *req);
 void complete_request();
 void init_network(char *mainSrcUrl);

graphics.c

@@ -10,7 +10,6 @@ window_proc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam)
     PAINTSTRUCT ps;
     HDC hdc;
     Request *req;
-    bool done;
 
     switch (uMsg) {
     case WM_CLOSE:
@@ -61,11 +60,8 @@ window_proc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam)
 
     if (uMsg == ioMsg) {
         req = (Request*)lParam;
-        done = handle_io_progress(req);
+        handle_io_progress(req);
         SetEvent(req->readyEvent);
-        if (done) {
-            free(req);
-        }
         return 0;
     }
 

requests.c

@@ -112,7 +112,7 @@ cancel_request(Request *req)
     req->failed = true;
 }
 
-bool
+void
 handle_io_progress(Request *req)
 {
     NPError err;
@@ -169,36 +169,16 @@ handle_io_progress(Request *req)
                 logmsg("destroystream error %d\n", err);
             }
             free(req->stream);
+            req->stream = NULL;
         }
 
         if (req->doNotify) {
             logmsg("> NPP_UrlNotify %s %d %p\n", req->originalUrl, req->doneReason, req->notifyData);
             pluginFuncs.urlnotify(&npp, req->originalUrl, req->doneReason, req->notifyData);
         }
 
-        if ((req->source == REQ_SRC_FILE || req->source == REQ_SRC_CACHE) && req->handles.hFile != INVALID_HANDLE_VALUE) {
-            CloseHandle(req->handles.hFile);
-        }
-
-        if (req->source == REQ_SRC_CACHE) {
-            UnlockUrlCacheEntryFileA(req->originalUrl, 0);
-        }
-
-        if (req->source == REQ_SRC_HTTP) {
-            if (req->handles.http.hReq != NULL) {
-                InternetCloseHandle(req->handles.http.hReq);
-            }
-            if (req->handles.http.hConn != NULL) {
-                InternetCloseHandle(req->handles.http.hConn);
-            }
-        }
-
         complete_request();
-
-        return true;
     }
-
-    return false;
 }
 
 void
@@ -570,28 +550,16 @@ VOID CALLBACK
 handle_request(PTP_CALLBACK_INSTANCE inst, void *reqArg, PTP_WORK work)
 {
     Request *req;
-    HANDLE doneEvent;
-    HANDLE readyEvent;
-    bool done;
 
     req = (Request *)reqArg;
-    doneEvent = req->doneEvent;
-    readyEvent = req->readyEvent;
-    done = false;
-    while (!done) {
+    while (true) {
         if (req->source == REQ_SRC_UNSET) {
             init_request(req);
         }
         if (!req->failed) {
             progress_request(req);
         }
 
-        /*
-         * Capture done state before posting to the main thread,
-         * since the main thread may free req after processing.
-         */
-        done = req->done || req->failed;
-
         if (req->hOutFile != INVALID_HANDLE_VALUE) {
             /* write directly to output file */
             DWORD written;
@@ -600,18 +568,40 @@ handle_request(PTP_CALLBACK_INSTANCE inst, void *reqArg, PTP_WORK work)
             }
             req->writePtr = req->writeSize;
         } else {
-            /* notify main thread to pipe progress to the plugin */
+            /* main thread has to pipe progress to the plugin */
             PostMessageW(hwnd, ioMsg, (WPARAM)NULL, (LPARAM)req);
-            WaitForSingleObject(readyEvent, INFINITE);
+            WaitForSingleObject(req->readyEvent, INFINITE);
+        }
+
+        if (req->done || req->failed) {
+            break;
+        }
+    }
+
+    /* cleanup I/O handles */
+    if ((req->source == REQ_SRC_FILE || req->source == REQ_SRC_CACHE)
+        && req->handles.hFile != INVALID_HANDLE_VALUE) {
+        CloseHandle(req->handles.hFile);
+    }
+    if (req->source == REQ_SRC_CACHE) {
+        UnlockUrlCacheEntryFileA(req->originalUrl, 0);
+    }
+    if (req->source == REQ_SRC_HTTP) {
+        if (req->handles.http.hReq != NULL) {
+            InternetCloseHandle(req->handles.http.hReq);
+        }
+        if (req->handles.http.hConn != NULL) {
+            InternetCloseHandle(req->handles.http.hConn);
         }
     }
 
-    /* Close handles here, because req has been freed at this point */
-    if (doneEvent != INVALID_HANDLE_VALUE) {
-        SetEvent(doneEvent);
-        CloseHandle(doneEvent);
+    /* cleanup signals */
+    if (req->doneEvent != INVALID_HANDLE_VALUE) {
+        SetEvent(req->doneEvent);
+        CloseHandle(req->doneEvent);
     }
-    CloseHandle(readyEvent);
+    CloseHandle(req->readyEvent);
+    free(req);
 }
 
 static void