chylex/Nextcloud-Desktop
1
0
Fork 0
mirror of https://github.com/chylex/Nextcloud-Desktop.git synced 2025-05-11 11:34:09 +02:00
Code

vfs: Be more careful about Vfs instance ownership

Browse Source
This commit is contained in:
Christian Kamm 2018-11-15 09:45:14 +01:00 committed by Kevin Ottens
parent 9196aa8e0a
commit fa2450cf11
No known key found for this signature in database
GPG Key ID: 074BBBCB8DECC9E2
7 changed files with 31 additions and 33 deletions

4
src/common/vfs.cpp
View File

@ -87,7 +87,7 @@ Vfs::Mode OCC::bestAvailableVfsMode()
Q_LOGGING_CATEGORY(lcPlugin, "plugins", QtInfoMsg)
Vfs *OCC::createVfsFromPlugin(Vfs::Mode mode, QObject *parent)
std::unique_ptr<Vfs> OCC::createVfsFromPlugin(Vfs::Mode mode)
{
auto name = modeToPluginName(mode);
if (name.isEmpty())
@ -107,7 +107,7 @@ Vfs *OCC::createVfsFromPlugin(Vfs::Mode mode, QObject *parent)
return nullptr;
}
auto vfs = qobject_cast<Vfs *>(factory->create(parent));
auto vfs = std::unique_ptr<Vfs>(qobject_cast<Vfs *>(factory->create(nullptr)));
if (!vfs) {
qCWarning(lcPlugin) << "Plugin" << pluginPath << "does not create a Vfs instance";
return nullptr;

4
src/common/vfs.h
View File

@ -17,6 +17,8 @@
#include <QScopedPointer>
#include <QSharedPointer>
#include <memory>
#include "ocsynclib.h"
#include "result.h"
@ -163,6 +165,6 @@ OCSYNC_EXPORT bool isVfsPluginAvailable(Vfs::Mode mode);
OCSYNC_EXPORT Vfs::Mode bestAvailableVfsMode();
/// Create a VFS instance for the mode, returns nullptr on failure.
OCSYNC_EXPORT Vfs *createVfsFromPlugin(Vfs::Mode mode, QObject *parent);
OCSYNC_EXPORT std::unique_ptr<Vfs> createVfsFromPlugin(Vfs::Mode mode);
} // namespace OCC

25
src/gui/folder.cpp
View File

@ -51,7 +51,7 @@ namespace OCC {
Q_LOGGING_CATEGORY(lcFolder, "nextcloud.gui.folder", QtInfoMsg)
Folder::Folder(const FolderDefinition &definition,
AccountState *accountState, Vfs *vfs,
AccountState *accountState, std::unique_ptr<Vfs> vfs,
QObject *parent)
: QObject(parent)
, _accountState(accountState)
@ -61,7 +61,7 @@ Folder::Folder(const FolderDefinition &definition,
, _consecutiveFollowUpSyncs(0)
, _journal(_definition.absoluteJournalPath())
, _fileLog(new SyncRunFileLog)
, _vfs(vfs)
, _vfs(vfs.release())
{
_timeSinceLastSyncStart.start();
_timeSinceLastSyncDone.start();
@ -120,13 +120,12 @@ Folder::Folder(const FolderDefinition &definition,
// Potentially upgrade suffix vfs to windows vfs
if (_definition.virtualFilesMode == Vfs::WithSuffix && _definition.upgradeVfsMode) {
if (auto winvfs = createVfsFromPlugin(Vfs::WindowsCfApi, this)) {
if (auto winvfs = createVfsFromPlugin(Vfs::WindowsCfApi)) {
// Wipe the existing suffix files from fs and journal
SyncEngine::wipeVirtualFiles(path(), _journal, _vfs);
SyncEngine::wipeVirtualFiles(path(), _journal, _vfs.data());
// Then switch to winvfs mode
delete _vfs;
_vfs = winvfs;
_vfs.reset(winvfs.release());
_definition.virtualFilesMode = Vfs::WindowsCfApi;
saveToSettings();
}
@ -473,8 +472,6 @@ void Folder::startVfs()
ENFORCE(_vfs);
ENFORCE(_vfs->mode() == _definition.virtualFilesMode);
_vfs->setParent(this);
VfsSetupParams vfsParams;
vfsParams.filesystemPath = path();
vfsParams.remotePath = remotePath();
@ -483,8 +480,8 @@ void Folder::startVfs()
vfsParams.providerName = Theme::instance()->appNameGUI();
vfsParams.providerVersion = Theme::instance()->version();
connect(_vfs, &OCC::Vfs::beginHydrating, this, &Folder::slotHydrationStarts);
connect(_vfs, &OCC::Vfs::doneHydrating, this, &Folder::slotHydrationDone);
connect(_vfs.data(), &OCC::Vfs::beginHydrating, this, &Folder::slotHydrationStarts);
connect(_vfs.data(), &OCC::Vfs::doneHydrating, this, &Folder::slotHydrationDone);
_vfs->registerFolder(vfsParams); // Do this always?
_vfs->start(vfsParams);
@ -600,7 +597,7 @@ void Folder::setUseVirtualFiles(bool enabled)
if (enabled && _definition.virtualFilesMode == Vfs::Off) {
_definition.virtualFilesMode = bestAvailableVfsMode();
_vfs = createVfsFromPlugin(_definition.virtualFilesMode, this);
_vfs.reset(createVfsFromPlugin(_definition.virtualFilesMode).release());
startVfs();
_saveInFoldersWithPlaceholders = true;
@ -609,11 +606,11 @@ void Folder::setUseVirtualFiles(bool enabled)
ENFORCE(_vfs);
// TODO: Must wait for current sync to finish!
SyncEngine::wipeVirtualFiles(path(), _journal, _vfs);
SyncEngine::wipeVirtualFiles(path(), _journal, _vfs.data());
_vfs->stop();
_vfs->unregisterFolder();
delete _vfs;
_vfs.reset(nullptr);
_definition.virtualFilesMode = Vfs::Off;
}
@ -802,7 +799,7 @@ void Folder::setSyncOptions()
opt._newBigFolderSizeLimit = newFolderLimit.first ? newFolderLimit.second * 1000LL * 1000LL : -1; // convert from MB to B
opt._confirmExternalStorage = cfgFile.confirmExternalStorage();
opt._moveFilesToTrash = cfgFile.moveToTrash();
opt._vfs = _vfs;
opt._vfs = _vfs.data();
QByteArray chunkSizeEnv = qgetenv("OWNCLOUD_CHUNK_SIZE");
if (!chunkSizeEnv.isEmpty()) {

7
src/gui/folder.h
View File

@ -28,6 +28,7 @@
#include <QUuid>
#include <set>
#include <chrono>
#include <memory>
class QThread;
class QSettings;
@ -102,10 +103,8 @@ class Folder : public QObject
public:
/** Create a new Folder
*
* The vfs instance will be parented to this.
*/
Folder(const FolderDefinition &definition, AccountState *accountState, Vfs *vfs, QObject *parent = nullptr);
Folder(const FolderDefinition &definition, AccountState *accountState, std::unique_ptr<Vfs> vfs, QObject *parent = nullptr);
~Folder();
@ -444,7 +443,7 @@ private:
/**
* The vfs mode instance (created by plugin) to use. Null means no vfs.
*/
Vfs *_vfs = nullptr;
QScopedPointer<Vfs> _vfs;
};
}

19
src/gui/folderman.cpp
View File

@ -259,12 +259,12 @@ void FolderMan::setupFoldersHelper(QSettings &settings, AccountStatePtr account,
qCInfo(lcFolderMan) << "Successfully migrated syncjournal database.";
}
Vfs *vfs = createVfsFromPlugin(folderDefinition.virtualFilesMode, nullptr);
auto vfs = createVfsFromPlugin(folderDefinition.virtualFilesMode);
if (!vfs && folderDefinition.virtualFilesMode != Vfs::Off) {
qCWarning(lcFolderMan) << "Could not load plugin for mode" << folderDefinition.virtualFilesMode;
}
Folder *f = addFolderInternal(folderDefinition, account.data(), vfs);
Folder *f = addFolderInternal(folderDefinition, account.data(), std::move(vfs));
f->saveToSettings();
continue;
@ -284,13 +284,13 @@ void FolderMan::setupFoldersHelper(QSettings &settings, AccountStatePtr account,
SyncJournalDb::maybeMigrateDb(folderDefinition.localPath, folderDefinition.absoluteJournalPath());
}
Vfs *vfs = createVfsFromPlugin(folderDefinition.virtualFilesMode, nullptr);
auto vfs = createVfsFromPlugin(folderDefinition.virtualFilesMode);
if (!vfs && folderDefinition.virtualFilesMode != Vfs::Off) {
// TODO: Must do better error handling
qFatal("Could not load plugin");
}
Folder *f = addFolderInternal(std::move(folderDefinition), account.data(), vfs);
Folder *f = addFolderInternal(std::move(folderDefinition), account.data(), std::move(vfs));
if (f) {
// Migration: Mark folders that shall be saved in a backwards-compatible way
if (backwardsCompatible)
@ -505,8 +505,7 @@ Folder *FolderMan::setupFolderFromOldConfigFile(const QString &file, AccountStat
folderDefinition.paused = paused;
folderDefinition.ignoreHiddenFiles = ignoreHiddenFiles();
Vfs *vfs = nullptr;
folder = addFolderInternal(folderDefinition, accountState, vfs);
folder = addFolderInternal(folderDefinition, accountState, std::unique_ptr<Vfs>());
if (folder) {
QStringList blackList = settings.value(QLatin1String("blackList")).toStringList();
if (!blackList.empty()) {
@ -994,13 +993,13 @@ Folder *FolderMan::addFolder(AccountState *accountState, const FolderDefinition
return nullptr;
}
Vfs *vfs = createVfsFromPlugin(folderDefinition.virtualFilesMode, nullptr);
auto vfs = createVfsFromPlugin(folderDefinition.virtualFilesMode);
if (!vfs && folderDefinition.virtualFilesMode != Vfs::Off) {
qCWarning(lcFolderMan) << "Could not load plugin for mode" << folderDefinition.virtualFilesMode;
return 0;
}
auto folder = addFolderInternal(definition, accountState, vfs);
auto folder = addFolderInternal(definition, accountState, std::move(vfs));
// Migration: The first account that's configured for a local folder shall
// be saved in a backwards-compatible way.
@ -1025,7 +1024,7 @@ Folder *FolderMan::addFolder(AccountState *accountState, const FolderDefinition
Folder *FolderMan::addFolderInternal(
FolderDefinition folderDefinition,
AccountState *accountState,
Vfs *vfs)
std::unique_ptr<Vfs> vfs)
{
auto alias = folderDefinition.alias;
int count = 0;
@ -1036,7 +1035,7 @@ Folder *FolderMan::addFolderInternal(
folderDefinition.alias = alias + QString::number(++count);
}
auto folder = new Folder(folderDefinition, accountState, vfs, this);
auto folder = new Folder(folderDefinition, accountState, std::move(vfs), this);
if (_navigationPaneHelper.showInExplorerNavigationPane() && folderDefinition.navigationPaneClsid.isNull()) {
folder->setNavigationPaneClsid(QUuid::createUuid());

2
src/gui/folderman.h
View File

@ -293,7 +293,7 @@ private:
* does not set an account on the new folder.
*/
Folder *addFolderInternal(FolderDefinition folderDefinition,
AccountState *accountState, Vfs *vfs);
AccountState *accountState, std::unique_ptr<Vfs> vfs);
/* unloads a folder object, does not delete it */
void unloadFolder(Folder *);

3
test/testsyncvirtualfiles.cpp
View File

@ -62,7 +62,8 @@ void markForDehydration(FakeFolder &folder, const QByteArray &path)
SyncOptions vfsSyncOptions()
{
SyncOptions options;
options._vfs = createVfsFromPlugin(Vfs::WithSuffix, nullptr);
// leak here
options._vfs = createVfsFromPlugin(Vfs::WithSuffix).release();
return options;
}