Skip to content

Commit

Permalink
Revert "Updating XPC interfaces to pass along context, and fixing som…
Browse files Browse the repository at this point in the history
…e retries (project-chip#35441)" (project-chip#35457)

This reverts commit f89d5b9.

Co-authored-by: Andrei Litvin <[email protected]>
  • Loading branch information
andy31415 and andreilitvin authored Sep 6, 2024
1 parent e96ddd9 commit 96e687b
Show file tree
Hide file tree
Showing 9 changed files with 91 additions and 270 deletions.
53 changes: 0 additions & 53 deletions src/darwin/Framework/CHIP/MTRDefines_Internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
8 changes: 1 addition & 7 deletions src/darwin/Framework/CHIP/MTRDeviceController+XPC.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/darwin/Framework/CHIP/MTRDeviceController_XPC.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
175 changes: 32 additions & 143 deletions src/darwin/Framework/CHIP/MTRDeviceController_XPC.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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

Expand All @@ -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
Expand All @@ -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:)
Expand All @@ -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
{
Expand Down Expand Up @@ -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;
}
}
Expand All @@ -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;
Expand All @@ -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;
}
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit 96e687b

Please sign in to comment.