From b6597c99192b34f956d63e6ecd6675d35318b5df Mon Sep 17 00:00:00 2001
From: Daniel Baston <dbaston@gmail.com>
Date: Tue, 6 Dec 2022 22:10:51 -0500
Subject: [PATCH] Isolate interrupt requests to single thread

---
 src/util/Interrupt.cpp                |   2 +-
 tests/unit/capi/GEOSInterruptTest.cpp |  50 +++++++++++++
 tests/unit/util/InterruptTest.cpp     | 102 ++++++++++++++++++++++++++
 3 files changed, 153 insertions(+), 1 deletion(-)
 create mode 100644 tests/unit/util/InterruptTest.cpp

diff --git a/src/util/Interrupt.cpp b/src/util/Interrupt.cpp
index 0bc988221b..2f3ba6c1fc 100644
--- a/src/util/Interrupt.cpp
+++ b/src/util/Interrupt.cpp
@@ -17,7 +17,7 @@
 
 namespace {
 /* Could these be portably stored in thread-specific space ? */
-bool requested = false;
+thread_local bool requested = false;
 
 geos::util::Interrupt::Callback* callback = nullptr;
 }
diff --git a/tests/unit/capi/GEOSInterruptTest.cpp b/tests/unit/capi/GEOSInterruptTest.cpp
index 0bd0301143..5b0e9ec71e 100644
--- a/tests/unit/capi/GEOSInterruptTest.cpp
+++ b/tests/unit/capi/GEOSInterruptTest.cpp
@@ -9,6 +9,7 @@
 #include <cstdio>
 #include <cstdlib>
 #include <memory>
+#include <thread>
 
 namespace tut {
 //
@@ -18,6 +19,7 @@ namespace tut {
 // Common data used in test cases.
 struct test_capiinterrupt_data {
     static int numcalls;
+    static int numcalls2;
     static GEOSInterruptCallback* nextcb;
 
     static void
@@ -56,9 +58,19 @@ struct test_capiinterrupt_data {
         }
     }
 
+    static void
+    countCalls2()
+    {
+        ++numcalls2;
+        if(nextcb) {
+            (*nextcb)();
+        }
+    }
+
 };
 
 int test_capiinterrupt_data::numcalls = 0;
+int test_capiinterrupt_data::numcalls2 = 0;
 GEOSInterruptCallback* test_capiinterrupt_data::nextcb = nullptr;
 
 typedef test_group<test_capiinterrupt_data> group;
@@ -221,5 +233,43 @@ void object::test<5>
 }
 
 
+// Test callback is thread-local
+template<>
+template<>
+void object::test<6>
+()
+{
+    numcalls = 0;
+    numcalls2 = 0;
+    nextcb = nullptr;
+
+    initGEOS(notice, notice);
+
+    auto buffer = [](GEOSInterruptCallback* cb) {
+        GEOSGeometry* geom1 = GEOSGeomFromWKT("LINESTRING (0 0, 1 0)");
+
+        GEOS_interruptRegisterCallback(cb);
+
+        GEOSGeometry* geom2 = GEOSBuffer(geom1, 1, 8);
+        GEOSGeom_destroy(geom2);
+        GEOSGeom_destroy(geom1);
+
+        GEOS_interruptRegisterCallback(nullptr);
+    };
+
+    std::thread t1(buffer, countCalls);
+    std::thread t2(buffer, countCalls2);
+
+    t1.join();
+    t2.join();
+
+    ensure("numcalls > 0", numcalls > 0);
+    ensure("numcalls2 > 0", numcalls2 > 0);
+    ensure_equals(numcalls, numcalls2);
+
+    finishGEOS();
+}
+
+
 } // namespace tut
 
diff --git a/tests/unit/util/InterruptTest.cpp b/tests/unit/util/InterruptTest.cpp
new file mode 100644
index 0000000000..6fb20146ea
--- /dev/null
+++ b/tests/unit/util/InterruptTest.cpp
@@ -0,0 +1,102 @@
+// tut
+#include <tut/tut.hpp>
+// geos
+#include <geos/util/Interrupt.h>
+// std
+#include <chrono>
+#include <thread>
+
+using geos::util::Interrupt;
+
+namespace tut {
+//
+// Test Group
+//
+
+// Common data used in test cases.
+struct test_interrupt_data {
+    static void workForever() {
+        try {
+            std::cerr << "Started " << std::this_thread::get_id() << "." << std::endl;
+            while (true) {
+                std::this_thread::sleep_for(std::chrono::milliseconds(10));
+                GEOS_CHECK_FOR_INTERRUPTS();
+            }
+        } catch (const std::exception&) {
+            std::cerr << "Interrupted " << std::this_thread::get_id() << "." << std::endl;
+            return;
+        }
+    }
+
+    static void interruptNow() {
+        Interrupt::request();
+    }
+
+    static std::map<std::thread::id, bool>* toInterrupt;
+
+    static void interruptIfRequested() {
+        if (toInterrupt == nullptr) {
+            return;
+        }
+
+        auto it = toInterrupt->find(std::this_thread::get_id());
+        if (it != toInterrupt->end() && it->second) {
+            it->second = false;
+            Interrupt::request();
+        }
+    }
+};
+
+std::map<std::thread::id, bool>* test_interrupt_data::toInterrupt = nullptr;
+
+typedef test_group<test_interrupt_data> group;
+typedef group::object object;
+
+group test_interrupt_group("geos::util::Interrupt");
+
+//
+// Test Cases
+//
+
+
+// Interrupt worker thread via global request from from main thead
+// No longer works, because Interrupt::request now interrupts only the thread that calls it.
+#if 0
+template<>
+template<>
+void object::test<1>
+()
+{
+    std::thread t(workForever);
+    Interrupt::request();
+
+    t.join();
+}
+#endif
+
+// Interrupt worker thread via global requset from worker thread using a callback
+template<>
+template<>
+void object::test<2>
+()
+{
+    Interrupt::registerCallback(interruptIfRequested);
+
+    std::thread t1(workForever);
+    std::thread t2(workForever);
+
+    std::map<std::thread::id, bool> shouldInterrupt;
+    shouldInterrupt[t1.get_id()] = false;
+    shouldInterrupt[t2.get_id()] = false;
+    toInterrupt = &shouldInterrupt;
+
+    shouldInterrupt[t2.get_id()] = true;
+    std::this_thread::sleep_for(std::chrono::milliseconds(1000));
+    shouldInterrupt[t1.get_id()] = true;
+
+    t1.join();
+    t2.join();
+}
+
+} // namespace tut
+