[stable-3.16] gui/macOS: Fix memory issues in Objective-C++ code for FileProvider support

src/gui/macOS/fileproviderdomainmanager_mac.mm

@@ -275,7 +275,6 @@ void addFileProviderDomain(const AccountState * const accountState)
 
             NSFileProviderDomain * const fileProviderDomain = [[NSFileProviderDomain alloc] initWithIdentifier:domainId.toNSString()
                                                                                                    displayName:domainDisplayName.toNSString()];
-            [fileProviderDomain retain];
 
             [NSFileProviderManager addDomain:fileProviderDomain completionHandler:^(NSError * const error) {
                 if(error) {

src/gui/macOS/fileproviderdomainsyncstatus_mac.mm

@@ -41,6 +41,7 @@ explicit MacImplementation(const QString &domainIdentifier, FileProviderDomainSy
             qCWarning(lcMacFileProviderDomainSyncStatus) << "Could not get manager for domain" << domainIdentifier;
             return;
         }
+        [_manager retain];
 
         if (@available(macOS 11.3, *)) {
             NSProgress *const downloadProgress = [_manager globalProgressForKind:NSProgressFileOperationKindDownloading];
@@ -61,6 +62,8 @@ explicit MacImplementation(const QString &domainIdentifier, FileProviderDomainSy
     {
         [_downloadProgressObserver release];
         [_uploadProgressObserver release];
+        [_domain release];
+        [_manager release];
     }
 
     void updateDownload(NSProgress *const progress) const

src/gui/macOS/fileprovidereditlocallyjob_mac.mm

@@ -44,8 +44,7 @@
 
     NSFileProviderDomain *const domain = (NSFileProviderDomain *)voidDomain;
     if (domain == nil) {
-        qCWarning(lcFileProviderEditLocallyMacJob) << "Could not get domain for account:"
-                                                   << userId;
+        qCWarning(lcFileProviderEditLocallyMacJob) << "Could not get domain for account:" << userId;
         emit notAvailable();
     }
 
@@ -56,37 +55,35 @@
         emit notAvailable();
     }
 
-    dispatch_semaphore_t semaphore = dispatch_semaphore_create(0);
-    __block NSError *receivedError;
-    __block NSURL *itemLocalUrl;
+    [manager retain];
     [manager getUserVisibleURLForItemIdentifier:nsOcId
                               completionHandler:^(NSURL *const url, NSError *const error) {
-        [url retain];
-        [error retain];
-        itemLocalUrl = url;
-        receivedError = error;
-        dispatch_semaphore_signal(semaphore);
-    }];
-    dispatch_semaphore_wait(semaphore, DISPATCH_TIME_FOREVER);
 
-    Systray::instance()->destroyEditFileLocallyLoadingDialog();
+        dispatch_async(dispatch_get_main_queue(), ^{
+            Systray::instance()->destroyEditFileLocallyLoadingDialog();
+        });
 
-    if (receivedError != nil) {
-        const auto errorMessage = QString::fromNSString(receivedError.localizedDescription);
-        qCWarning(lcFileProviderEditLocallyMacJob) << "Error getting user visible URL for item"
-                                                   << ocId << ":" << errorMessage;
-        emit notAvailable();
-    } else if (itemLocalUrl != nil) {
-        const auto itemLocalPath = QString::fromNSString(itemLocalUrl.path);
-        qCDebug(lcFileProviderEditLocallyMacJob) << "Got user visible URL for item" 
-                                                 << ocId << ":" << itemLocalPath;
-        [NSWorkspace.sharedWorkspace openURL:itemLocalUrl];
-        emit finished();
-    } else {
-        qCWarning(lcFileProviderEditLocallyMacJob) << "Got nil user visible URL for item"
-                                                   << ocId;
-        emit notAvailable();
-    }
+        if (error != nil) {
+            const auto errorMessage = QString::fromNSString(error.localizedDescription);
+            qCWarning(lcFileProviderEditLocallyMacJob) << "Error getting user visible URL for item:" << errorMessage;
+            dispatch_async(dispatch_get_main_queue(), ^{
+                emit notAvailable();
+            });
+        } else if (url != nil) {
+            const auto itemLocalPath = QString::fromNSString(url.path);
+            qCDebug(lcFileProviderEditLocallyMacJob) << "Got user visible URL for item:" << itemLocalPath;
+            [NSWorkspace.sharedWorkspace openURL:url];
+            dispatch_async(dispatch_get_main_queue(), ^{
+                emit finished();
+            });
+        } else {
+            qCWarning(lcFileProviderEditLocallyMacJob) << "Got nil user visible URL for item" << ocId;
+            dispatch_async(dispatch_get_main_queue(), ^{
+                emit notAvailable();
+            });
+        }
+        [manager release];
+    }];
 }
 
 } // namespace OCC::Mac

src/gui/macOS/fileprovideritemmetadata_mac.mm

@@ -120,9 +120,9 @@ QString nsNameComponentsToLocalisedQString(NSPersonNameComponents *const nameCom
     }
 
     __block QString returnPath = QObject::tr("Unknown");
-    NSFileProviderManager *manager = FileProviderUtils::managerForDomainIdentifier(domainId);
+    NSFileProviderManager *const manager = FileProviderUtils::managerForDomainIdentifier(domainId);
 
-     if (manager == nil) {
+    if (manager == nil) {
         qCWarning(lcMacImplFileProviderItemMetadata) << "Null manager, cannot get item path";
         return returnPath;
     }
@@ -132,6 +132,7 @@ QString nsNameComponentsToLocalisedQString(NSPersonNameComponents *const nameCom
 
     // getUserVisibleUrl is async, so wait here
 
+    [manager retain];
     [manager getUserVisibleURLForItemIdentifier:nsItemIdentifier
                               completionHandler:^(NSURL *const userVisibleFile, NSError *const error) {
 
@@ -141,6 +142,7 @@ QString nsNameComponentsToLocalisedQString(NSPersonNameComponents *const nameCom
             returnPath = QString::fromNSString(userVisibleFile.path);
         }
 
+        [manager release];
         dispatch_semaphore_signal(semaphore);
     }];
 

src/gui/macOS/fileprovidermaterialiseditemsmodel_mac.mm

@@ -30,7 +30,7 @@
 
 void FileProviderMaterialisedItemsModel::evictItem(const QString &identifier, const QString &domainIdentifier)
 {
-    NSFileProviderManager * const manager = FileProviderUtils::managerForDomainIdentifier(domainIdentifier);
+    NSFileProviderManager *const manager = FileProviderUtils::managerForDomainIdentifier(domainIdentifier);
     if (manager == nil) {
         qCWarning(lcMacImplFileProviderMaterialisedItemsModelMac) << "Received null manager for domain"
                                                                   << domainIdentifier
@@ -42,7 +42,8 @@
         return;
     }
 
-    __block BOOL successfullyDeleted = YES;
+    __block BOOL successfullyDeleted = NO;
+    dispatch_semaphore_t semaphore = dispatch_semaphore_create(0);
 
     [manager evictItemWithIdentifier:identifier.toNSString() completionHandler:^(NSError *error) {
         if (error != nil) {
@@ -51,10 +52,15 @@
             Systray::instance()->showMessage(tr("Error"),
                                              tr("An error occurred while trying to delete the local copy of this item: %1").arg(errorDesc),
                                              QSystemTrayIcon::Warning);
-            successfullyDeleted = NO;
+        } else {
+            successfullyDeleted = YES;
         }
+        dispatch_semaphore_signal(semaphore);
     }];
 
+    dispatch_semaphore_wait(semaphore, dispatch_time(DISPATCH_TIME_NOW, 3 * NSEC_PER_SEC));
+    [manager release];
+
     if (successfullyDeleted == NO) {
         return;
     }

src/gui/macOS/fileprovidersettingscontroller_mac.mm

@@ -167,6 +167,7 @@ void signalFileProviderDomain(const QString &userIdAtHost) const
         qCInfo(lcFileProviderSettingsController) << "Signalling file provider domain" << userIdAtHost;
         NSFileProviderDomain * const domain = FileProviderUtils::domainForIdentifier(userIdAtHost);
         NSFileProviderManager * const manager = [NSFileProviderManager managerForDomain:domain];
+        [domain release];
         [manager signalEnumeratorForContainerItemIdentifier:NSFileProviderRootContainerItemIdentifier
                                           completionHandler:^(NSError *const error) {
             if (error != nil) {
@@ -186,6 +187,7 @@ void signalFileProviderDomain(const QString &userIdAtHost) const
     }
 
 public slots:
+    // NOTE: This method will release the provided args so make sure to retain them beforehand
     void enumerateMaterialisedFilesForDomainManager(NSFileProviderManager * const managerForDomain,
                                                     NSFileProviderDomain * const domain)
     {
@@ -194,7 +196,6 @@ void enumerateMaterialisedFilesForDomainManager(NSFileProviderManager * const ma
         [enumerator retain];
 
         FileProviderStorageUseEnumerationObserver *const storageUseObserver = [[FileProviderStorageUseEnumerationObserver alloc] init];
-        [storageUseObserver retain];
         storageUseObserver.enumerationFinishedHandler = ^(NSError *const error) {
             qCInfo(lcFileProviderSettingsController) << "Enumeration finished for" << domain.identifier;
             if (error != nil) {
@@ -229,6 +230,9 @@ void enumerateMaterialisedFilesForDomainManager(NSFileProviderManager * const ma
 
             [storageUseObserver release];
             [enumerator release];
+
+            [managerForDomain release];
+            [domain release];
         };
         [enumerator enumerateItemsForObserver:storageUseObserver startingAtPage:NSFileProviderInitialPageSortedByName];
     }
@@ -284,7 +288,8 @@ void fetchMaterialisedFilesStorageUsage()
                                                                 << ", returning early.";
                     return;
                 }
-
+                [managerForDomain retain];
+                [domain retain];
                 enumerateMaterialisedFilesForDomainManager(managerForDomain, domain);
             }
         }];

src/gui/macOS/fileproviderutils.h

@@ -29,6 +29,10 @@ class QString;
  *
  * You should threfore try to avoid using this in C++ code wherever possible
  * and only use this in *_mac.mm implementation files.
+ *
+ * IMPORTANT: All Objective-C objects returned here need to be released!
+ * They have been internally retained due to the async nature of the
+ * FileProvider API.
  */
 
 namespace OCC {

src/gui/macOS/fileproviderxpc_mac.mm

@@ -148,6 +148,8 @@
     } else {
         qCWarning(lcFileProviderXPC) << "Could not open debug log file" << filename;
     }
+
+    [rcvdDebugLogString release];
 }
 
 bool FileProviderXPC::fileProviderExtReachable(const QString &extensionAccountId, const bool retry, const bool reconfigureOnFail)

src/gui/macOS/fileproviderxpc_mac_utils.h

@@ -29,6 +29,6 @@ NSArray<NSXPCConnection *> *connectToFileProviderServices(NSArray<NSDictionary<N
 void configureFileProviderConnection(NSXPCConnection *connection);
 NSObject *getRemoteServiceObject(NSXPCConnection *connection, Protocol *protocol);
 NSString *getExtensionAccountId(NSObject<ClientCommunicationProtocol> *clientCommService);
-QHash<QString, void*> processClientCommunicationConnections(NSArray *connections);
+QHash<QString, void*> processClientCommunicationConnections(NSArray<NSXPCConnection *> *connections);
 
 }

src/gui/macOS/fileproviderxpc_mac_utils.mm

@@ -30,7 +30,7 @@
 NSArray<NSFileProviderManager *> *getDomainManagers()
 {
     dispatch_group_t group = dispatch_group_create();
-    __block NSMutableArray<NSFileProviderManager *> *managers = NSMutableArray.array;
+    __block NSMutableArray<NSFileProviderManager *> *const managers = NSMutableArray.array;
 
     dispatch_group_enter(group);
 
@@ -45,8 +45,11 @@
         for (NSFileProviderDomain *const domain in domains) {
             qCInfo(lcFileProviderXPCUtils) << "Got domain" << domain.identifier;
             NSFileProviderManager *const manager = [NSFileProviderManager managerForDomain:domain];
-            [manager retain];
-            [managers addObject:manager];
+            if (manager) {
+                [managers addObject:manager];
+            } else {
+                qCWarning(lcFileProviderXPCUtils) << "Could not get manager for domain" << domain.identifier;
+            }
         }
 
         dispatch_group_leave(group);
@@ -78,7 +81,6 @@
                 } else if (service == nil) {
                     qCWarning(lcFileProviderXPCUtils) << "Service is nil";
                 } else {
-                    [service retain];
                     [fpServices addObject:@{service.name: service}];
                 }
                 dispatch_group_leave(group);
@@ -95,7 +97,7 @@
 NSArray<NSURL *> *getDomainUrlsForManagers(NSArray<NSFileProviderManager *> *managers)
 {
     dispatch_group_t group = dispatch_group_create();
-    __block NSMutableArray<NSURL *> *urls = NSMutableArray.array;
+    __block NSMutableArray<NSURL *> *const urls = NSMutableArray.array;
 
     for (NSFileProviderManager *const manager in managers) {
 
@@ -191,7 +193,6 @@
                     return;
                 }
 
-                [connection retain];
                 [connections addObject:connection];
                 dispatch_group_leave(group);
             }];
@@ -243,15 +244,14 @@ void configureFileProviderConnection(NSXPCConnection *const connection)
             dispatch_group_leave(group);
             return;
         }
-        extensionNcAccount = [NSString stringWithString:extensionAccountId];
-        [extensionNcAccount retain];
+        extensionNcAccount = [[NSString alloc] initWithString:extensionAccountId];
         dispatch_group_leave(group);
     }];
     dispatch_group_wait(group, DISPATCH_TIME_FOREVER);
     return extensionNcAccount;
 }
 
-QHash<QString, void*> processClientCommunicationConnections(NSArray *const connections)
+QHash<QString, void*> processClientCommunicationConnections(NSArray<NSXPCConnection *> *const connections)
 {
     QHash<QString, void*> clientCommServices;
 

src/gui/updater/sparkleupdater_mac.mm

@@ -248,13 +248,10 @@ - (NSString *)feedURLStringForUpdater:(SPUUpdater *)updater
     , _interface(std::make_unique<SparkleInterface>(this))
 {
     _interface->delegate = [[NCSparkleUpdaterDelegate alloc] initWithOwner:_interface.get()];
-    [_interface->delegate retain];
-
     _interface->updaterController =
         [[SPUStandardUpdaterController alloc] initWithStartingUpdater:YES 
                                                       updaterDelegate:_interface->delegate
                                                    userDriverDelegate:nil];
-    [_interface->updaterController retain];
 
     setUpdateUrl(appCastUrl);