Make work contract cold paths use locks, hot paths use atomics.

Author: Geenz

src/Concurrency/WorkContractGroup.cpp

@@ -206,11 +206,15 @@ namespace Concurrency {
         ENTROPY_ASSERT(mainThreadSelectingCount == 0, "WorkContractGroup destroyed with threads still in selectForMainThreadExecution");
 
         // Then notify the concurrency provider to remove us from active groups
+        // CRITICAL: Read provider without holding lock to avoid ABBA deadlock
+        IConcurrencyProvider* provider = nullptr;
         {
             std::unique_lock<std::shared_mutex> lock(_concurrencyProviderMutex);
-            if (_concurrencyProvider) {
-                _concurrencyProvider->notifyGroupDestroyed(this);
-            }
+            provider = _concurrencyProvider;
+        }
+
+        if (provider) {
+            provider->notifyGroupDestroyed(this);
         }
     }
 

src/Concurrency/WorkContractGroup.h

@@ -29,6 +29,7 @@
 #include <thread>
 #include <condition_variable>
 #include <mutex>
+#include <shared_mutex>
 #include <optional>
 #include <limits>
 
@@ -176,9 +177,22 @@ namespace Concurrency {
         /**
          * @brief Destructor ensures all work is stopped and completed
          * 
-         * Calls stop() to prevent new work selection, then wait() to ensure
-         * all executing work completes. Finally notifies the concurrency provider
-         * (if any) that the group is being destroyed.
+         * Follows a strict destruction protocol to prevent deadlocks:
+         * 1. Calls stop() to prevent new work selection
+         * 2. Calls wait() to ensure all executing work completes
+         * 3. Unschedules and releases all remaining contracts
+         * 4. Reads concurrency provider pointer WITHOUT holding mutex lock
+         * 5. Calls notifyGroupDestroyed() to inform provider of destruction
+         *
+         * CRITICAL: The provider notification is made without holding the group's
+         * concurrency provider mutex to prevent ABBA deadlock with WorkService.
+         * Any deviation from this protocol may result in deadlock during destruction.
+         *
+         * The provider will then:
+         * - Remove this group from its internal lists
+         * - Call setConcurrencyProvider(nullptr) to clear the back-reference
+         *
+         * This ensures proper bidirectional cleanup without lock ordering issues.
          */
         ~WorkContractGroup();