Revert "Updating XPC interfaces to pass along context, and fixing some retries (project-chip#35441)" (project-chip#35457)

This reverts commit f89d5b9.

Co-authored-by: Andrei Litvin <andreilitvin@google.com>

Author: andy31415

src/darwin/Framework/CHIP/MTRDefines_Internal.h

@@ -111,56 +111,3 @@ typedef struct {} variable_hidden_by_mtr_hide;
                                                                                                                     \
         return outValue;                                                                                            \
     }
-
-#ifndef MTR_OPTIONAL_ATTRIBUTE
-#if __has_feature(objc_arc)
-#define MTR_OPTIONAL_ATTRIBUTE(ATTRIBUTE, VALUE, DICTIONARY)                                                                                       \
-    {                                                                                                                                              \
-        id valueToAdd = VALUE;                                                                                                                     \
-        if (valueToAdd != nil) {                                                                                                                   \
-            CFDictionarySetValue((CFMutableDictionaryRef) DICTIONARY, (CFStringRef) (__bridge const void *) ATTRIBUTE, (const void *) valueToAdd); \
-        }                                                                                                                                          \
-    }
-#else
-#define MTR_OPTIONAL_ATTRIBUTE(ATTRIBUTE, VALUE, DICTIONARY)                                                                              \
-    {                                                                                                                                     \
-        id valueToAdd = VALUE;                                                                                                            \
-        if (valueToAdd != nil) {                                                                                                          \
-            CFDictionarySetValue((CFMutableDictionaryRef) DICTIONARY, (CFStringRef) (const void *) ATTRIBUTE, (const void *) valueToAdd); \
-        }                                                                                                                                 \
-    }
-#endif
-#endif
-
-#ifndef MTR_OPTIONAL_COLLECTION_ATTRIBUTE
-#define MTR_OPTIONAL_COLLECTION_ATTRIBUTE(ATTRIBUTE, COLLECTION, DICTIONARY)                                           \
-    if ([COLLECTION count] > 0) {                                                                                      \
-        CFDictionarySetValue((CFMutableDictionaryRef) DICTIONARY, (CFStringRef) ATTRIBUTE, (const void *) COLLECTION); \
-    }
-#endif
-
-#ifndef MTR_OPTIONAL_NUMBER_ATTRIBUTE
-#define MTR_OPTIONAL_NUMBER_ATTRIBUTE(ATTRIBUTE, NUMBER, DICTIONARY)                                               \
-    if ([NUMBER intValue] != 0) {                                                                                  \
-        CFDictionarySetValue((CFMutableDictionaryRef) DICTIONARY, (CFStringRef) ATTRIBUTE, (const void *) NUMBER); \
-    }
-#endif
-
-#ifndef MTR_REMOVE_ATTRIBUTE
-#define MTR_REMOVE_ATTRIBUTE(ATTRIBUTE, DICTIONARY)                                            \
-    if (ATTRIBUTE != nil && DICTIONARY) {                                                      \
-        CFDictionaryRemoveValue((CFMutableDictionaryRef) DICTIONARY, (CFStringRef) ATTRIBUTE); \
-    }
-#endif
-
-#ifndef MTR_REQUIRED_ATTRIBUTE
-#define MTR_REQUIRED_ATTRIBUTE(ATTRIBUTE, VALUE, DICTIONARY)                                                               \
-    {                                                                                                                      \
-        id valueToAdd = VALUE;                                                                                             \
-        if (valueToAdd != nil) {                                                                                           \
-            CFDictionarySetValue((CFMutableDictionaryRef) DICTIONARY, (CFStringRef) ATTRIBUTE, (const void *) valueToAdd); \
-        } else {                                                                                                           \
-            MTR_LOG_ERROR("Warning, missing %@ to add to %s", ATTRIBUTE, #DICTIONARY);                                     \
-        }                                                                                                                  \
-    }
-#endif

src/darwin/Framework/CHIP/MTRDeviceController+XPC.mm

@@ -34,13 +34,7 @@
 static NSSet * GetXPCAllowedClasses()
 {
     static NSSet * const sXPCAllowedClasses = [NSSet setWithArray:@[
-        [NSString class],
-        [NSNumber class],
-        [NSData class],
-        [NSArray class],
-        [NSDictionary class],
-        [NSError class],
-        [NSDate class],
+        [NSString class], [NSNumber class], [NSData class], [NSArray class], [NSDictionary class], [NSError class]
     ]];
     return sXPCAllowedClasses;
 }

src/darwin/Framework/CHIP/MTRDeviceController_XPC.h

@@ -27,7 +27,7 @@ MTR_TESTABLE
 - (id)initWithUniqueIdentifier:(NSUUID *)UUID machServiceName:(NSString *)machServiceName options:(NSXPCConnectionOptions)options
 #endif
 
-    @property(nullable, atomic, retain, readwrite)NSXPCConnection * xpcConnection;
+    @property(atomic, retain, readwrite)NSXPCConnection * xpcConnection;
 
 @end
 

src/darwin/Framework/CHIP/MTRDeviceController_XPC.mm

@@ -19,7 +19,6 @@
 #import "MTRDefines_Internal.h"
 #import "MTRDeviceController_Internal.h"
 #import "MTRDevice_XPC.h"
-#import "MTRDevice_XPC_Internal.h"
 #import "MTRLogging_Internal.h"
 #import "MTRXPCClientProtocol.h"
 #import "MTRXPCServerProtocol.h"
@@ -34,10 +33,7 @@
 
 @interface MTRDeviceController_XPC ()
 
-@property (nonatomic, readwrite, retain) NSUUID * uniqueIdentifier;
-@property (nonnull, atomic, readwrite, retain) MTRXPCDeviceControllerParameters * xpcParameters;
-@property (atomic, readwrite, assign) NSTimeInterval xpcRetryTimeInterval;
-@property (atomic, readwrite, assign) BOOL xpcConnectedOrConnecting;
+@property (nonatomic, retain, readwrite) NSUUID * uniqueIdentifier;
 
 @end
 
@@ -52,15 +48,7 @@ - (NSXPCInterface *)_interfaceForServerProtocol
     NSXPCInterface * interface = [NSXPCInterface interfaceWithProtocol:@protocol(MTRXPCServerProtocol)];
 
     NSSet * allowedClasses = [NSSet setWithArray:@[
-        [NSString class],
-        [NSNumber class],
-        [NSData class],
-        [NSArray class],
-        [NSDictionary class],
-        [NSError class],
-        [MTRCommandPath class],
-        [MTRAttributePath class],
-        [NSDate class],
+        [NSString class], [NSNumber class], [NSData class], [NSArray class], [NSDictionary class], [NSError class], [MTRCommandPath class], [MTRAttributePath class]
     ]];
 
     [interface setClasses:allowedClasses
@@ -74,14 +62,7 @@ - (NSXPCInterface *)_interfaceForClientProtocol
 {
     NSXPCInterface * interface = [NSXPCInterface interfaceWithProtocol:@protocol(MTRXPCClientProtocol)];
     NSSet * allowedClasses = [NSSet setWithArray:@[
-        [NSString class],
-        [NSNumber class],
-        [NSData class],
-        [NSArray class],
-        [NSDictionary class],
-        [NSError class],
-        [MTRAttributePath class],
-        [NSDate class],
+        [NSString class], [NSNumber class], [NSData class], [NSArray class], [NSDictionary class], [NSError class], [MTRAttributePath class]
     ]];
     [interface setClasses:allowedClasses
               forSelector:@selector(device:receivedAttributeReport:)
@@ -102,114 +83,6 @@ - (NSXPCInterface *)_interfaceForClientProtocol
     return [self.uniqueIdentifier UUIDString];
 }
 
-- (void)_startXPCConnectionRetry
-{
-    if (!self.xpcConnectedOrConnecting) {
-        MTR_LOG("%@: XPC Connection retry - Starting retry for XPC Connection", self);
-        self.xpcRetryTimeInterval = 0.5;
-        mtr_weakify(self);
-
-        dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t) (self.xpcRetryTimeInterval * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
-            mtr_strongify(self);
-            [self _xpcConnectionRetry];
-        });
-    } else {
-        MTR_LOG("%@: XPC Connection retry - Not starting retry for XPC Connection, already trying", self);
-    }
-}
-
-- (void)_xpcConnectionRetry
-{
-    MTR_LOG("%@: XPC Connection retry - timer hit", self);
-    if (!self.xpcConnectedOrConnecting) {
-        if (![self _setupXPCConnection]) {
-#if 0 // FIXME: Not sure why this retry is not working, but I will fix this later
-            MTR_LOG("%@: XPC Connection retry - Scheduling another retry", self);
-            self.xpcRetryTimeInterval = self.xpcRetryTimeInterval >= 1 ? self.xpcRetryTimeInterval * 2 : 1;
-            self.xpcRetryTimeInterval = MIN(60.0, self.xpcRetryTimeInterval);
-            mtr_weakify(self);
-
-            dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(self.xpcRetryTimeInterval * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
-                mtr_strongify(self);
-                [self _xpcConnectionRetry];
-            });
-#else
-            MTR_LOG("%@: XPC Connection failed retry - bailing", self);
-#endif
-        } else {
-            MTR_LOG("%@: XPC Connection retry - connection attempt successful", self);
-        }
-    } else {
-        MTR_LOG("%@: XPC Connection retry - Mid retry, or connected, stopping retry timer", self);
-    }
-}
-
-- (BOOL)_setupXPCConnection
-{
-    self.xpcConnection = self.xpcParameters.xpcConnectionBlock();
-
-    MTR_LOG("%@ Set up XPC Connection: %@", self, self.xpcConnection);
-    if (self.xpcConnection) {
-        mtr_weakify(self);
-        self.xpcConnection.remoteObjectInterface = [self _interfaceForServerProtocol];
-
-        self.xpcConnection.exportedInterface = [self _interfaceForClientProtocol];
-        self.xpcConnection.exportedObject = self;
-
-        self.xpcConnection.interruptionHandler = ^{
-            mtr_strongify(self);
-            MTR_LOG_ERROR("XPC Connection for device controller interrupted: %@", self.xpcParameters.uniqueIdentifier);
-            self.xpcConnectedOrConnecting = NO;
-            self.xpcConnection = nil;
-            [self _startXPCConnectionRetry];
-        };
-
-        self.xpcConnection.invalidationHandler = ^{
-            mtr_strongify(self);
-            MTR_LOG_ERROR("XPC Connection for device controller invalidated: %@", self.xpcParameters.uniqueIdentifier);
-            self.xpcConnectedOrConnecting = NO;
-            self.xpcConnection = nil;
-            [self _startXPCConnectionRetry];
-        };
-
-        MTR_LOG("%@ Activating new XPC connection", self);
-        [self.xpcConnection activate];
-
-        [[self.xpcConnection synchronousRemoteObjectProxyWithErrorHandler:^(NSError * _Nonnull error) {
-            MTR_LOG_ERROR("Checkin error: %@", error);
-        }] deviceController:self.uniqueIdentifier checkInWithContext:[NSDictionary dictionary]];
-
-        // FIXME: Trying to kick all the MTRDevices attached to this controller to re-establish connections
-        //        This state needs to be stored properly and re-established at connnection time
-
-        MTR_LOG("%@ Starting existing NodeID Registration", self);
-        for (NSNumber * nodeID in [self.nodeIDToDeviceMap keyEnumerator]) {
-            MTR_LOG("%@ => Registering nodeID: %@", self, nodeID);
-            mtr_weakify(self);
-
-            [[self.xpcConnection synchronousRemoteObjectProxyWithErrorHandler:^(NSError * _Nonnull error) {
-                mtr_strongify(self);
-                MTR_LOG_ERROR("%@ Registration error for device nodeID: %@ : %@", self, nodeID, error);
-            }] deviceController:self.uniqueIdentifier registerNodeID:nodeID];
-        }
-
-        __block BOOL barrierComplete = NO;
-
-        [self.xpcConnection scheduleSendBarrierBlock:^{
-            barrierComplete = YES;
-            MTR_LOG("%@: Barrier complete: %d", self, barrierComplete);
-        }];
-
-        MTR_LOG("%@ Done existing NodeID Registration, barrierComplete: %d", self, barrierComplete);
-        self.xpcConnectedOrConnecting = barrierComplete;
-    } else {
-        MTR_LOG_ERROR("%@ Failed to set up XPC Connection", self);
-        self.xpcConnectedOrConnecting = NO;
-    }
-
-    return (self.xpcConnectedOrConnecting);
-}
-
 - (nullable instancetype)initWithParameters:(MTRDeviceControllerAbstractParameters *)parameters
                                       error:(NSError * __autoreleasing *)error
 {
@@ -237,12 +110,30 @@ - (nullable instancetype)initWithParameters:(MTRDeviceControllerAbstractParamete
             return nil;
         }
 
-        self.xpcParameters = xpcParameters;
+        self.xpcConnection = connectionBlock();
+        self.uniqueIdentifier = UUID;
         self.chipWorkQueue = dispatch_queue_create("MTRDeviceController_XPC_queue", DISPATCH_QUEUE_SERIAL_WITH_AUTORELEASE_POOL);
         self.nodeIDToDeviceMap = [NSMapTable strongToWeakObjectsMapTable];
-        self.uniqueIdentifier = UUID;
 
-        if (![self _setupXPCConnection]) {
+        MTR_LOG("Set up XPC Connection: %@", self.xpcConnection);
+        if (self.xpcConnection) {
+            self.xpcConnection.remoteObjectInterface = [self _interfaceForServerProtocol];
+
+            self.xpcConnection.exportedInterface = [self _interfaceForClientProtocol];
+            self.xpcConnection.exportedObject = self;
+
+            self.xpcConnection.interruptionHandler = ^{
+                MTR_LOG_ERROR("XPC Connection for device controller interrupted: %@", UUID);
+            };
+
+            self.xpcConnection.invalidationHandler = ^{
+                MTR_LOG_ERROR("XPC Connection for device controller invalidated: %@", UUID);
+            };
+
+            MTR_LOG("Activating new XPC connection");
+            [self.xpcConnection activate];
+        } else {
+            MTR_LOG_ERROR("Failed to set up XPC Connection");
             return nil;
         }
     }
@@ -268,7 +159,7 @@ - (id)initWithUniqueIdentifier:(NSUUID *)UUID machServiceName:(NSString *)machSe
             self.xpcConnection.exportedObject = self;
 
             MTR_LOG("%s: resuming new XPC connection");
-            [self.xpcConnection activate];
+            [self.xpcConnection resume];
         } else {
             MTR_LOG_ERROR("Failed to set up XPC Connection");
             return nil;
@@ -286,7 +177,13 @@ - (MTRDevice *)_setupDeviceForNodeID:(NSNumber *)nodeID prefetchedClusterData:(N
     os_unfair_lock_assert_owner(self.deviceMapLock);
 
     MTRDevice * deviceToReturn = [[MTRDevice_XPC alloc] initWithNodeID:nodeID controller:self];
-    [self.nodeIDToDeviceMap setObject:deviceToReturn forKey:nodeID];
+    // If we're not running, don't add the device to our map.  That would
+    // create a cycle that nothing would break.  Just return the device,
+    // which will be in exactly the state it would be in if it were created
+    // while we were running and then we got shut down.
+    if ([self isRunning]) {
+        [self.nodeIDToDeviceMap setObject:deviceToReturn forKey:nodeID];
+    }
     MTR_LOG("%s: returning XPC device for node id %@", __PRETTY_FUNCTION__, nodeID);
     return deviceToReturn;
 }
@@ -358,14 +255,6 @@ - (oneway void)deviceConfigurationChanged:(NSNumber *)nodeID
     [device deviceConfigurationChanged:nodeID];
 }
 
-- (oneway void)device:(NSNumber *)nodeID internalStateUpdated:(NSDictionary *)dictionary
-{
-    MTRDevice_XPC * device = (MTRDevice_XPC *) [self deviceForNodeID:nodeID];
-    MTR_LOG("Received internalStateUpdated: %@ found device: %@", nodeID, device);
-
-    [device device:nodeID internalStateUpdated:dictionary];
-}
-
 #pragma mark - MTRDeviceController Protocol Client
 
 // Not Supported via XPC