Merge pull request #8080 from nextcloud/bugfix/objcpp-mem

gui/macOS: Fix memory issues in Objective-C++ code for FileProvider support
This commit is contained in:
Claudio Cambra 2025-03-27 15:15:58 +01:00 committed by GitHub
commit 092c8d774c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 66 additions and 51 deletions

View File

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

View File

@ -41,6 +41,7 @@ public:
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 @@ public:
{
[_downloadProgressObserver release];
[_uploadProgressObserver release];
[_domain release];
[_manager release];
}
void updateDownload(NSProgress *const progress) const

View File

@ -44,8 +44,7 @@ void FileProviderEditLocallyJob::openFileProviderFile(const QString &ocId)
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 @@ void FileProviderEditLocallyJob::openFileProviderFile(const QString &ocId)
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_async(dispatch_get_main_queue(), ^{
Systray::instance()->destroyEditFileLocallyLoadingDialog();
});
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];
}];
dispatch_semaphore_wait(semaphore, DISPATCH_TIME_FOREVER);
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();
}
}
} // namespace OCC::Mac

View File

@ -120,9 +120,9 @@ QString FileProviderItemMetadata::getUserVisiblePath() const
}
__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 FileProviderItemMetadata::getUserVisiblePath() const
// getUserVisibleUrl is async, so wait here
[manager retain];
[manager getUserVisibleURLForItemIdentifier:nsItemIdentifier
completionHandler:^(NSURL *const userVisibleFile, NSError *const error) {
@ -141,6 +142,7 @@ QString FileProviderItemMetadata::getUserVisiblePath() const
returnPath = QString::fromNSString(userVisibleFile.path);
}
[manager release];
dispatch_semaphore_signal(semaphore);
}];

View File

@ -30,7 +30,7 @@ Q_LOGGING_CATEGORY(lcMacImplFileProviderMaterialisedItemsModelMac, "nextcloud.gu
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 @@ void FileProviderMaterialisedItemsModel::evictItem(const QString &identifier, co
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 @@ void FileProviderMaterialisedItemsModel::evictItem(const QString &identifier, co
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;
}

View File

@ -167,6 +167,7 @@ public:
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 @@ public:
}
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 @@ public slots:
[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 @@ public slots:
[storageUseObserver release];
[enumerator release];
[managerForDomain release];
[domain release];
};
[enumerator enumerateItemsForObserver:storageUseObserver startingAtPage:NSFileProviderInitialPageSortedByName];
}
@ -284,7 +288,8 @@ private:
<< ", returning early.";
return;
}
[managerForDomain retain];
[domain retain];
enumerateMaterialisedFilesForDomainManager(managerForDomain, domain);
}
}];

View File

@ -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 {

View File

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

View File

@ -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);
}

View File

@ -30,7 +30,7 @@ Q_LOGGING_CATEGORY(lcFileProviderXPCUtils, "nextcloud.gui.macos.fileprovider.xpc
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 @@ NSArray<NSFileProviderManager *> *getDomainManagers()
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 @@ NSArray<NSDictionary<NSFileProviderServiceName, NSFileProviderService *> *> *get
} 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<NSDictionary<NSFileProviderServiceName, NSFileProviderService *> *> *get
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 @@ NSArray<NSXPCConnection *> *connectToFileProviderServices(NSArray<NSDictionary<N
return;
}
[connection retain];
[connections addObject:connection];
dispatch_group_leave(group);
}];
@ -243,15 +244,14 @@ NSString *getExtensionAccountId(NSObject<ClientCommunicationProtocol> *const cli
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;

View File

@ -248,13 +248,10 @@ SparkleUpdater::SparkleUpdater(const QUrl& appCastUrl)
, _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);