From 458e91e48b40c5f8f2e5aa002b0ac71eb46bb07f Mon Sep 17 00:00:00 2001
From: Nico Rieck <nico.rieck@gmail.com>
Date: Thu, 4 Feb 2021 18:11:08 +0100
Subject: [PATCH 1/2] Fix reliability issues in the parallelize tool

* output_collecting_pipe's destructor could hang on exit because it did not
  clear the running flag before cancelling. The threadpool callback would
  then start another IO operation which never completes.
* Use a manual reset event for overlapped IO to prevent potential memory
  corruption. AFAICT waiting on threadpool IO with named pipe handles is
  unreliable and may result in the kernel writing to a free'd OVERLAPPED
  struct.
---
 tools/inc/stljobs.h | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/tools/inc/stljobs.h b/tools/inc/stljobs.h
index a538a9e8456..10993ed4cfd 100644
--- a/tools/inc/stljobs.h
+++ b/tools/inc/stljobs.h
@@ -123,6 +123,16 @@ inline handle<invalid_handle_value_policy> create_file(LPCWSTR lpFileName, DWORD
     return result;
 }
 
+inline handle<null_handle_policy> create_event(
+    LPSECURITY_ATTRIBUTES lpEventAttributes, BOOL bManualReset, BOOL bInitialState, LPCWSTR lpName) {
+    handle<null_handle_policy> result{CreateEventW(lpEventAttributes, bManualReset, bInitialState, lpName)};
+    if (!result) {
+        api_failure("CreateEventW");
+    }
+
+    return result;
+}
+
 inline handle<invalid_handle_value_policy> create_named_pipe(LPCWSTR lpName, DWORD dwOpenMode, DWORD dwPipeMode,
     DWORD nMaxInstances, DWORD nOutBufferSize, DWORD nInBufferSize, DWORD nDefaultTimeOut,
     LPSECURITY_ATTRIBUTES lpSecurityAttributes) {
@@ -305,6 +315,9 @@ struct output_collecting_pipe {
         writeHandle = create_file(pipeNameBuffer, GENERIC_WRITE | FILE_READ_ATTRIBUTES, 0, &inheritSa, OPEN_EXISTING,
             FILE_FLAG_OVERLAPPED, HANDLE{});
 
+        readEvent         = create_event(nullptr, TRUE, FALSE, nullptr);
+        overlapped.hEvent = readEvent.get();
+
         readIo = tp_io{std::move(readHandle), callback, this, nullptr};
 
         start();
@@ -312,7 +325,7 @@ struct output_collecting_pipe {
 
     ~output_collecting_pipe() noexcept {
         if (readIo) {
-            if (running.load()) {
+            if (running.exchange(false)) {
                 if (!CancelIoEx(readIo.get_file(), &overlapped)) {
                     api_failure("CancelIoEx"); // slams into noexcept
                 }
@@ -413,6 +426,7 @@ struct output_collecting_pipe {
     std::string targetBuffer; // if running, owned by a threadpool thread, otherwise owned by the calling thread
     size_t validTill{};
     handle<invalid_handle_value_policy> writeHandle;
+    handle<null_handle_policy> readEvent;
     tp_io readIo;
     OVERLAPPED overlapped{};
 };

From 82094882fc254968c0b150dfd506506d35e25253 Mon Sep 17 00:00:00 2001
From: "Stephan T. Lavavej" <stl@nuwen.net>
Date: Sat, 13 Feb 2021 16:18:17 -0800
Subject: [PATCH 2/2] Add comment to running.exchange(false).

---
 tools/inc/stljobs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/inc/stljobs.h b/tools/inc/stljobs.h
index 10993ed4cfd..9167cc60860 100644
--- a/tools/inc/stljobs.h
+++ b/tools/inc/stljobs.h
@@ -325,7 +325,7 @@ struct output_collecting_pipe {
 
     ~output_collecting_pipe() noexcept {
         if (readIo) {
-            if (running.exchange(false)) {
+            if (running.exchange(false)) { // prevent callback() from calling read_some()
                 if (!CancelIoEx(readIo.get_file(), &overlapped)) {
                     api_failure("CancelIoEx"); // slams into noexcept
                 }