Enable "are we on the right thread?" asserts on Darwin. (#20177)

A few changes here:

1) Enable chip_stack_lock_tracking on darwin.
2) Implement _IsChipStackLockedByCurrentThread on Darwin in a way that makes
   sense with the actual "no locks" setup there (by checking that we are on the
   right dispatch queue).
3) Fix controller shutdown on Darwin to not happen from outside the Matter event
   loop while the event loop is running.

Removing the dealloc method from the Darwin controller implementation is safe
because:

* The only place where controllers get allocated is MTRControllerFactory's
  createController.
* At that callsite, if initWithFactory returns nil it guarantees that it has
  called cleanup.
* After allocation + init, we add the controller to the factory's controller
  list.  After that the controller cannot be deallocated until we remove it from
  that list, which only happens in controllerShuttingDown, which is always
  followed by a call to cleanup.

src/darwin/Framework/CHIP/CHIPDeviceController.mm

@@ -125,14 +125,21 @@ - (void)cleanupAfterStartup
     [self cleanup];
 }
 
+// Part of cleanupAfterStartup that has to interact with the Matter work queue
+// in a very specific way that only MTRControllerFactory knows about.
+- (void)shutDownCppController
+{
+    if (_cppCommissioner) {
+        _cppCommissioner->Shutdown();
+        delete _cppCommissioner;
+        _cppCommissioner = nullptr;
+    }
+}
+
 // Clean up any members we might have allocated.
 - (void)cleanup
 {
-    if (self->_cppCommissioner) {
-        self->_cppCommissioner->Shutdown();
-        delete self->_cppCommissioner;
-        self->_cppCommissioner = nullptr;
-    }
+    VerifyOrDie(_cppCommissioner == nullptr);
 
     [self clearDeviceAttestationDelegateBridge];
 
@@ -675,11 +682,6 @@ - (BOOL)checkForError:(CHIP_ERROR)errorCode logMsg:(NSString *)logMsg error:(NSE
     return YES;
 }
 
-- (void)dealloc
-{
-    [self cleanup];
-}
-
 - (BOOL)deviceBeingCommissionedOverBLE:(uint64_t)deviceId
 {
     CHIP_ERROR errorCode = CHIP_ERROR_INCORRECT_STATE;

src/darwin/Framework/CHIP/CHIPDeviceController_Internal.h

@@ -80,6 +80,14 @@ NS_ASSUME_NONNULL_BEGIN
                     fabricIndex:(chip::FabricIndex)fabricIndex
                       isRunning:(BOOL *)isRunning;
 
+/**
+ * Shut down the underlying C++ controller.  Must be called on the Matter work
+ * queue or after the Matter work queue has been shut down.
+ *
+ * Only MTRControllerFactory should be calling this.
+ */
+- (void)shutDownCppController;
+
 @end
 
 NS_ASSUME_NONNULL_END

src/darwin/Framework/CHIP/MatterControllerFactory.mm

@@ -525,6 +525,13 @@ - (void)controllerShuttingDown:(CHIPDeviceController *)controller
         // shuts down, because shutdown of the last controller will tear
         // down most of the world.
         DeviceLayer::PlatformMgrImpl().StopEventLoopTask();
+
+        [controller shutDownCppController];
+    } else {
+        // Do the controller shutdown on the Matter work queue.
+        dispatch_sync(_chipWorkQueue, ^{
+            [controller shutDownCppController];
+        });
     }
 }
 

src/platform/BUILD.gn

@@ -73,7 +73,7 @@ if (chip_device_platform != "none" && chip_device_platform != "external") {
   if (chip_stack_lock_tracking == "auto") {
     if (chip_device_platform == "linux" || chip_device_platform == "tizen" ||
         chip_device_platform == "android" || current_os == "freertos" ||
-        chip_device_platform == "webos") {
+        chip_device_platform == "webos" || chip_device_platform == "darwin") {
       # TODO: should be fatal for development. Change once bugs are fixed
       chip_stack_lock_tracking = "fatal"
     } else {

src/platform/Darwin/PlatformManagerImpl.cpp

@@ -69,9 +69,9 @@ CHIP_ERROR PlatformManagerImpl::_InitChipStack()
 
 CHIP_ERROR PlatformManagerImpl::_StartEventLoopTask()
 {
-    if (mIsWorkQueueRunning == false)
+    if (mIsWorkQueueSuspended)
     {
-        mIsWorkQueueRunning = true;
+        mIsWorkQueueSuspended = false;
         dispatch_resume(mWorkQueue);
     }
 
@@ -80,16 +80,19 @@ CHIP_ERROR PlatformManagerImpl::_StartEventLoopTask()
 
 CHIP_ERROR PlatformManagerImpl::_StopEventLoopTask()
 {
-    if (mIsWorkQueueRunning == true)
+    if (!mIsWorkQueueSuspended && !mIsWorkQueueSuspensionPending)
     {
-        mIsWorkQueueRunning = false;
+        mIsWorkQueueSuspensionPending = true;
         if (dispatch_get_current_queue() != mWorkQueue)
         {
             // dispatch_sync is used in order to guarantee serialization of the caller with
             // respect to any tasks that might already be on the queue, or running.
             dispatch_sync(mWorkQueue, ^{
                 dispatch_suspend(mWorkQueue);
             });
+
+            mIsWorkQueueSuspended         = true;
+            mIsWorkQueueSuspensionPending = false;
         }
         else
         {
@@ -99,6 +102,8 @@ CHIP_ERROR PlatformManagerImpl::_StopEventLoopTask()
             // that no more tasks will run on the queue.
             dispatch_async(mWorkQueue, ^{
                 dispatch_suspend(mWorkQueue);
+                mIsWorkQueueSuspended         = true;
+                mIsWorkQueueSuspensionPending = false;
                 dispatch_semaphore_signal(mRunLoopSem);
             });
         }
@@ -138,5 +143,14 @@ CHIP_ERROR PlatformManagerImpl::_PostEvent(const ChipDeviceEvent * event)
     return CHIP_NO_ERROR;
 }
 
+#if CHIP_STACK_LOCK_TRACKING_ENABLED
+bool PlatformManagerImpl::_IsChipStackLockedByCurrentThread() const
+{
+    // If we have no work queue, or it's suspended, then we assume our caller
+    // knows what they are doing in terms of their own concurrency.
+    return !mWorkQueue || mIsWorkQueueSuspended || dispatch_get_current_queue() == mWorkQueue;
+};
+#endif
+
 } // namespace DeviceLayer
 } // namespace chip

src/platform/Darwin/PlatformManagerImpl.h

@@ -49,6 +49,7 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener
         {
             mWorkQueue = dispatch_queue_create(CHIP_CONTROLLER_QUEUE, DISPATCH_QUEUE_SERIAL);
             dispatch_suspend(mWorkQueue);
+            mIsWorkQueueSuspended = true;
         }
         return mWorkQueue;
     }
@@ -71,7 +72,7 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener
     CHIP_ERROR _PostEvent(const ChipDeviceEvent * event);
 
 #if CHIP_STACK_LOCK_TRACKING_ENABLED
-    bool _IsChipStackLockedByCurrentThread() const { return false; };
+    bool _IsChipStackLockedByCurrentThread() const;
 #endif
 
     // ===== Members for internal use by the following friends.
@@ -88,7 +89,11 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener
     // Semaphore used to implement blocking behavior in _RunEventLoop.
     dispatch_semaphore_t mRunLoopSem;
 
-    bool mIsWorkQueueRunning = false;
+    bool mIsWorkQueueSuspended = false;
+    // TODO: mIsWorkQueueSuspensionPending might need to be an atomic and use
+    // atomic ops, if we're worried about calls to StopEventLoopTask() from
+    // multiple threads racing somehow...
+    bool mIsWorkQueueSuspensionPending = false;
 
     inline ImplClass * Impl() { return static_cast<PlatformManagerImpl *>(this); }
 };