Thread Sanitizer: This method should not be called on the main thread as it may lead to UI unresponsiveness.

[billymeltdown]

Hello, I have been seeing this warning from Thread Sanitizer when testing our Dropbox integration in the latest Xcode 14.3:

This method should not be called on the main thread as it may lead to UI unresponsiveness.

Our integration has been around for some time, so the problem is new to us, but it's possible the last few versions of Xcode were already highlighting it and I'm just noticing it now, having turned the Thread Sanitizer setting back on recently.

It seems to happen when unlinking clients, and when handling token response.

When we call [DBClientsManager unlinkAndResetClients] and it calls through DBOAuthManager into DBSDKKeychain's -clearAllTokens, the warning pops on the SecItemDelete() call:

+ (BOOL)clearAllTokens {
// According to Apple documentation, SecItemDelete should delete all matching items by default.
// The default behavior works fine on iOS, but not on macOS.
// An extra parameter is required to be able to delete all items on macOS, but this same parameter
// would result in an error on iOS. So only add it on macOS.
#if TARGET_OS_OSX
  NSMutableDictionary<id, id> *query = [DBSDKKeychain queryWithDict:@{(id)kSecMatchLimit : (id)kSecMatchLimitAll}];
#else
  NSMutableDictionary<id, id> *query = [DBSDKKeychain queryWithDict:@{}];
#endif
  return SecItemDelete((__bridge CFDictionaryRef)query) == noErr;
}

That's easy enough to resolve, we can wrap the call [DBClientsManager unlinkAndResetClients] in a dispatch_async to a background queue.

The tougher one to solve is when we are completing the oauth/token authentication redirect and calling handleRedirectURL:completion: on DBClientsManager. The main thread warning is thrown for both the SecItemDelete and SecItemAdd calls here, in DBSDKKeychain:

+ (BOOL)storeDataValueWithKey:(NSString *)key value:(NSData *)value {
  NSMutableDictionary<id, id> *query =
      [DBSDKKeychain queryWithDict:@{(id)kSecAttrAccount : key, (id)kSecValueData : value}];
  SecItemDelete((__bridge CFDictionaryRef)query);
  return SecItemAdd((__bridge CFDictionaryRef)query, nil) == noErr;
}

I could wrap handleRedirectURL:completion: in another background queue block, but DBOAuthTokenRequest, which is handling the response explicitly jumps back to the main queue here:

- (void)db_handleResponse:(NSURLResponse *)response data:(NSData *)data error:(NSError *)error {
#pragma unused(response)
  DBOAuthResult *result = nil;
  if (error) {
    // Network error
    result = [DBOAuthResult unknownErrorWithErrorDescription:error.localizedDescription];
  } else {
    // No network error, parse response data
    NSDictionary<NSString *, id> *resultDict = [self db_resultDictionaryFromData:data];
    result = [self db_extractResultFromDict:resultDict];
  }

  dispatch_queue_t queue = _queue ?: dispatch_get_main_queue();
  dispatch_async(queue, ^{
    self->_completion(result);
  });
  _retainSelf = nil;
}

I don't see a good way to specify a different _queue for this class to use, at least not from DBClientsManager, but the capability is there.

Does anyone have any advice on how I could better resolve this myself (e.g. don't use DBClientsManager?), or is this something that needs to be handled upstream in the dropbox-sdk-obj-c project? It's not causing any noticeable slow down in the interface, so it's not any kind of major issue I need to solve right away, but it is on my maintenance list so I thought I'd inquire.

Thanks for your time!

[greg-db]

Thanks for the detailed report! I'm raising this with the team to look into this.