NOISSUE retry committing instances if it fails a few times

This should fix issues with antivirus locking files/folders on Windows.

api/logic/FolderInstanceProvider.cpp

@@ -12,6 +12,7 @@
 #include <QJsonObject>
 #include <QJsonArray>
 #include <QUuid>
+#include <QTimer>
 
 const static int GROUP_FILE_FORMAT_VERSION = 1;
 
@@ -110,24 +111,6 @@ InstancePtr FolderInstanceProvider::loadInstance(const InstanceId& id)
 	return inst;
 }
 
-#include "InstanceImportTask.h"
-Task * FolderInstanceProvider::zipImportTask(const QUrl sourceUrl, const QString& instName, const QString& instGroup, const QString& instIcon)
-{
-	return new InstanceImportTask(m_globalSettings, sourceUrl, this, instName, instIcon, instGroup);
-}
-
-#include "InstanceCreationTask.h"
-Task * FolderInstanceProvider::creationTask(BaseVersionPtr version, const QString& instName, const QString& instGroup, const QString& instIcon)
-{
-	return new InstanceCreationTask(m_globalSettings, this, version, instName, instIcon, instGroup);
-}
-
-#include "InstanceCopyTask.h"
-Task * FolderInstanceProvider::copyTask(const InstancePtr& oldInstance, const QString& instName, const QString& instGroup, const QString& instIcon, bool copySaves)
-{
-	return new InstanceCopyTask(m_globalSettings, this, oldInstance, instName, instIcon, instGroup, copySaves);
-}
-
 void FolderInstanceProvider::saveGroupList()
 {
 	WatchLock foo(m_watcher, m_instDir);
@@ -310,12 +293,65 @@ void FolderInstanceProvider::on_InstFolderChanged(const Setting &setting, QVaria
 		emit instancesChanged();
 	}
 }
+
+template <typename T>
+static void clamp(T& current, T min, T max)
+{
+	if (current < min)
+	{
+		current = min;
+	}
+	else if(current > max)
+	{
+		current = max;
+	}
+}
+
+// List of numbers from min to max. Next is exponent times bigger than previous.
+class ExponentialSeries
+{
+public:
+	ExponentialSeries(unsigned min, unsigned max, unsigned exponent = 2)
+	{
+		m_current = m_min = min;
+		m_max = max;
+		m_exponent = exponent;
+	}
+	void reset()
+	{
+		m_current = m_min;
+	}
+	unsigned operator()()
+	{
+		unsigned retval = m_current;
+		m_current *= m_exponent;
+		clamp(m_current, m_min, m_max);
+		return retval;
+	}
+	unsigned m_current;
+	unsigned m_min;
+	unsigned m_max;
+	unsigned m_exponent;
+};
+
 /*
+ * WHY: the whole reason why this uses an exponential backoff retry scheme is antivirus on Windows.
+ * Basically, it starts messing things up while MultiMC is extracting/creating instances
+ * and causes that horrible failure that is NTFS to lock files in place because they are open.
+ */
 class FolderInstanceStaging : public Task
 {
-
+Q_OBJECT
+	const unsigned minBackoff = 1;
+	const unsigned maxBackoff = 16;
 public:
-	FolderInstanceStaging(FolderInstanceProvider * parent, Task * child, const QString& instanceName, const QString& groupName)
+	FolderInstanceStaging (
+		FolderInstanceProvider * parent,
+		Task * child,
+		const QString & stagingPath,
+		const QString& instanceName,
+		const QString& groupName )
+	: backoff(minBackoff, maxBackoff)
 	{
 		m_parent = parent;
 		m_child.reset(child);
@@ -325,22 +361,34 @@ public:
 		connect(child, &Task::progress, this, &FolderInstanceStaging::setProgress);
 		m_instanceName = instanceName;
 		m_groupName = groupName;
+		m_stagingPath = stagingPath;
+		m_backoffTimer.setSingleShot(true);
+		connect(&m_backoffTimer, &QTimer::timeout, this, &FolderInstanceStaging::childSucceded);
 	}
 
 protected:
 	virtual void executeTask() override
 	{
-		m_stagingPath = m_parent->getStagedInstancePath();
 		m_child->start();
 	}
 
 private slots:
 	void childSucceded()
 	{
+		unsigned sleepTime = backoff();
 		if(m_parent->commitStagedInstance(m_stagingPath, m_instanceName, m_groupName))
+		{
 			emitSucceeded();
-		// TODO: implement exponential backoff retry scheme with limit
+			return;
-		emitFailed("Failed to commit instance");
+		}
+		// we actually failed, retry?
+		if(sleepTime == maxBackoff)
+		{
+			emitFailed(tr("Failed to commit instance, even after multiple retries. It is being blocked by something."));
+			return;
+		}
+		qDebug() << "Failed to commit instance" << m_instanceName << "Initiating backoff:" << sleepTime;
+		m_backoffTimer.start(sleepTime * 500);
 	}
 	void childFailed(const QString & reason)
 	{
@@ -349,13 +397,38 @@ private slots:
 	}
 
 private:
+	ExponentialSeries backoff;
 	QString m_stagingPath;
 	FolderInstanceProvider * m_parent;
 	unique_qobject_ptr<Task> m_child;
 	QString m_instanceName;
 	QString m_groupName;
+	QTimer m_backoffTimer;
 };
-*/
+
+#include "InstanceImportTask.h"
+Task * FolderInstanceProvider::zipImportTask(const QUrl sourceUrl, const QString& instName, const QString& instGroup, const QString& instIcon)
+{
+	auto stagingPath = getStagedInstancePath();
+	auto task = new InstanceImportTask(m_globalSettings, sourceUrl, stagingPath, instName, instIcon, instGroup);
+	return new FolderInstanceStaging(this, task, stagingPath, instName, instGroup);
+}
+
+#include "InstanceCreationTask.h"
+Task * FolderInstanceProvider::creationTask(BaseVersionPtr version, const QString& instName, const QString& instGroup, const QString& instIcon)
+{
+	auto stagingPath = getStagedInstancePath();
+	auto task = new InstanceCreationTask(m_globalSettings, stagingPath, version, instName, instIcon, instGroup);
+	return new FolderInstanceStaging(this, task, stagingPath, instName, instGroup);
+}
+
+#include "InstanceCopyTask.h"
+Task * FolderInstanceProvider::copyTask(const InstancePtr& oldInstance, const QString& instName, const QString& instGroup, const QString& instIcon, bool copySaves)
+{
+	auto stagingPath = getStagedInstancePath();
+	auto task = new InstanceCopyTask(m_globalSettings, stagingPath, oldInstance, instName, instIcon, instGroup, copySaves);
+	return new FolderInstanceStaging(this, task, stagingPath, instName, instGroup);
+}
 
 QString FolderInstanceProvider::getStagedInstancePath()
 {
@@ -395,3 +468,4 @@ bool FolderInstanceProvider::destroyStagingPath(const QString& keyPath)
 	return FS::deletePath(keyPath);
 }
 
+#include "FolderInstanceProvider.moc"

api/logic/InstanceCopyTask.cpp

@@ -6,10 +6,10 @@
 #include "pathmatcher/RegexpMatcher.h"
 #include <QtConcurrentRun>
 
-InstanceCopyTask::InstanceCopyTask(SettingsObjectPtr settings, BaseInstanceProvider* target, InstancePtr origInstance, const QString& instName, const QString& instIcon, const QString& instGroup, bool copySaves)
+InstanceCopyTask::InstanceCopyTask(SettingsObjectPtr settings, const QString & stagingPath, InstancePtr origInstance, const QString& instName, const QString& instIcon, const QString& instGroup, bool copySaves)
 {
 	m_globalSettings = settings;
-	m_target = target;
+	m_stagingPath = stagingPath;
 	m_origInstance = origInstance;
 	m_instName = instName;
 	m_instIcon = instIcon;
@@ -27,7 +27,7 @@ InstanceCopyTask::InstanceCopyTask(SettingsObjectPtr settings, BaseInstanceProvi
 void InstanceCopyTask::executeTask()
 {
 	setStatus(tr("Copying instance %1").arg(m_origInstance->name()));
-	m_stagingPath = m_target->getStagedInstancePath();
+
 	FS::copy folderCopy(m_origInstance->instanceRoot(), m_stagingPath);
 	folderCopy.followSymlinks(false).blacklist(m_matcher.get());
 
@@ -42,7 +42,6 @@ void InstanceCopyTask::copyFinished()
 	auto successful = m_copyFuture.result();
 	if(!successful)
 	{
-		m_target->destroyStagingPath(m_stagingPath);
 		emitFailed(tr("Instance folder copy failed."));
 		return;
 	}
@@ -56,13 +55,11 @@ void InstanceCopyTask::copyFinished()
 	InstancePtr inst(new NullInstance(m_globalSettings, instanceSettings, m_stagingPath));
 	inst->setName(m_instName);
 	inst->setIconKey(m_instIcon);
-	m_target->commitStagedInstance(m_stagingPath, m_instName, m_instGroup);
 	emitSucceeded();
 }
 
 void InstanceCopyTask::copyAborted()
 {
-	m_target->destroyStagingPath(m_stagingPath);
 	emitFailed(tr("Instance folder copy has been aborted."));
 	return;
 }

api/logic/InstanceCopyTask.h

@@ -17,7 +17,7 @@ class MULTIMC_LOGIC_EXPORT InstanceCopyTask : public Task
 {
 	Q_OBJECT
 public:
-	explicit InstanceCopyTask(SettingsObjectPtr settings, BaseInstanceProvider * target, InstancePtr origInstance, const QString &instName,
+	explicit InstanceCopyTask(SettingsObjectPtr settings, const QString & stagingPath, InstancePtr origInstance, const QString &instName,
 		const QString &instIcon, const QString &instGroup, bool copySaves);
 
 protected:
@@ -28,7 +28,6 @@ protected:
 
 private: /* data */
 	SettingsObjectPtr m_globalSettings;
-	BaseInstanceProvider * m_target = nullptr;
 	InstancePtr m_origInstance;
 	QString m_instName;
 	QString m_instIcon;

api/logic/InstanceCreationTask.cpp

@@ -6,11 +6,11 @@
 //FIXME: remove this
 #include "minecraft/onesix/OneSixInstance.h"
 
-InstanceCreationTask::InstanceCreationTask(SettingsObjectPtr settings, BaseInstanceProvider* target, BaseVersionPtr version,
+InstanceCreationTask::InstanceCreationTask(SettingsObjectPtr settings, const QString & stagingPath, BaseVersionPtr version,
 	const QString& instName, const QString& instIcon, const QString& instGroup)
 {
 	m_globalSettings = settings;
-	m_target = target;
+	m_stagingPath = stagingPath;
 	m_instName = instName;
 	m_instIcon = instIcon;
 	m_instGroup = instGroup;
@@ -20,19 +20,17 @@ InstanceCreationTask::InstanceCreationTask(SettingsObjectPtr settings, BaseInsta
 void InstanceCreationTask::executeTask()
 {
 	setStatus(tr("Creating instance from version %1").arg(m_version->name()));
-
+	{
-	QString stagingPath = m_target->getStagedInstancePath();
+		auto instanceSettings = std::make_shared<INISettingsObject>(FS::PathCombine(m_stagingPath, "instance.cfg"));
-	QDir rootDir(stagingPath);
+		instanceSettings->suspendSave();
-
+		instanceSettings->registerSetting("InstanceType", "Legacy");
-	auto instanceSettings = std::make_shared<INISettingsObject>(FS::PathCombine(stagingPath, "instance.cfg"));
+		instanceSettings->set("InstanceType", "OneSix");
-	instanceSettings->registerSetting("InstanceType", "Legacy");
+		OneSixInstance inst(m_globalSettings, instanceSettings, m_stagingPath);
-
+		inst.setIntendedVersionId(m_version->descriptor());
-	instanceSettings->set("InstanceType", "OneSix");
+		inst.setName(m_instName);
-	InstancePtr inst(new OneSixInstance(m_globalSettings, instanceSettings, stagingPath));
+		inst.setIconKey(m_instIcon);
-	inst->setIntendedVersionId(m_version->descriptor());
+		inst.init();
-	inst->setName(m_instName);
+		instanceSettings->resumeSave();
-	inst->setIconKey(m_instIcon);
+	}
-	inst->init();
-	m_target->commitStagedInstance(stagingPath, m_instName, m_instGroup);
 	emitSucceeded();
 }

api/logic/InstanceCreationTask.h

@@ -7,13 +7,11 @@
 #include "settings/SettingsObject.h"
 #include "BaseVersion.h"
 
-class BaseInstanceProvider;
-
 class MULTIMC_LOGIC_EXPORT InstanceCreationTask : public Task
 {
 	Q_OBJECT
 public:
-	explicit InstanceCreationTask(SettingsObjectPtr settings, BaseInstanceProvider * target, BaseVersionPtr version, const QString &instName,
+	explicit InstanceCreationTask(SettingsObjectPtr settings, const QString & stagingPath, BaseVersionPtr version, const QString &instName,
 		const QString &instIcon, const QString &instGroup);
 
 protected:
@@ -22,7 +20,7 @@ protected:
 
 private: /* data */
 	SettingsObjectPtr m_globalSettings;
-	BaseInstanceProvider * m_target;
+	QString m_stagingPath;
 	BaseVersionPtr m_version;
 	QString m_instName;
 	QString m_instIcon;

api/logic/InstanceImportTask.cpp

@@ -16,12 +16,12 @@
 #include "minecraft/flame/PackManifest.h"
 #include "Json.h"
 
-InstanceImportTask::InstanceImportTask(SettingsObjectPtr settings, const QUrl sourceUrl, BaseInstanceProvider * target,
+InstanceImportTask::InstanceImportTask(SettingsObjectPtr settings, const QUrl sourceUrl, const QString & stagingPath,
 	const QString &instName, const QString &instIcon, const QString &instGroup)
 {
 	m_globalSettings = settings;
 	m_sourceUrl = sourceUrl;
-	m_target = target;
+	m_stagingPath = stagingPath;
 	m_instName = instName;
 	m_instIcon = instIcon;
 	m_instGroup = instGroup;
@@ -72,30 +72,9 @@ void InstanceImportTask::downloadProgressChanged(qint64 current, qint64 total)
 	setProgress(current / 2, total);
 }
 
-static QFileInfo findRecursive(const QString &dir, const QString &name)
-{
-	for (const auto info : QDir(dir).entryInfoList(QDir::NoDotAndDotDot | QDir::Dirs | QDir::Files, QDir::DirsLast))
-	{
-		if (info.isFile() && info.fileName() == name)
-		{
-			return info;
-		}
-		else if (info.isDir())
-		{
-			const QFileInfo res = findRecursive(info.absoluteFilePath(), name);
-			if (res.isFile() && res.exists())
-			{
-				return res;
-			}
-		}
-	}
-	return QFileInfo();
-}
-
 void InstanceImportTask::processZipPack()
 {
 	setStatus(tr("Extracting modpack"));
-	m_stagingPath = m_target->getStagedInstancePath();
 	QDir extractDir(m_stagingPath);
 	qDebug() << "Attempting to create instance from" << m_archivePath;
 
@@ -143,7 +122,6 @@ void InstanceImportTask::extractFinished()
 	m_packZip.reset();
 	if (m_extractFuture.result().isEmpty())
 	{
-		m_target->destroyStagingPath(m_stagingPath);
 		emitFailed(tr("Failed to extract modpack"));
 		return;
 	}
@@ -189,7 +167,6 @@ void InstanceImportTask::extractFinished()
 			processMultiMC();
 			return;
 		case ModpackType::Unknown:
-			m_target->destroyStagingPath(m_stagingPath);
 			emitFailed(tr("Archive does not contain a recognized modpack type."));
 			return;
 	}
@@ -197,7 +174,6 @@ void InstanceImportTask::extractFinished()
 
 void InstanceImportTask::extractAborted()
 {
-	m_target->destroyStagingPath(m_stagingPath);
 	emitFailed(tr("Instance import has been aborted."));
 	return;
 }
@@ -218,7 +194,6 @@ void InstanceImportTask::processFlame()
 	}
 	catch (JSONValidationError & e)
 	{
-		m_target->destroyStagingPath(m_stagingPath);
 		emitFailed(tr("Could not understand pack manifest:\n") + e.cause());
 		return;
 	}
@@ -228,7 +203,6 @@ void InstanceImportTask::processFlame()
 		QString mcPath = FS::PathCombine(m_stagingPath, "minecraft");
 		if (!QFile::rename(overridePath, mcPath))
 		{
-			m_target->destroyStagingPath(m_stagingPath);
 			emitFailed(tr("Could not rename the overrides folder:\n") + pack.overrides);
 			return;
 		}
@@ -331,18 +305,11 @@ void InstanceImportTask::processFlame()
 		connect(m_filesNetJob.get(), &NetJob::succeeded, this, [&]()
 		{
 			m_filesNetJob.reset();
-			if (!m_target->commitStagedInstance(m_stagingPath, m_instName, m_instGroup))
-			{
-				m_target->destroyStagingPath(m_stagingPath);
-				emitFailed(tr("Unable to commit instance"));
-				return;
-			}
 			emitSucceeded();
 		}
 		);
 		connect(m_filesNetJob.get(), &NetJob::failed, [&](QString reason)
 		{
-			m_target->destroyStagingPath(m_stagingPath);
 			m_filesNetJob.reset();
 			emitFailed(reason);
 		});
@@ -356,7 +323,6 @@ void InstanceImportTask::processFlame()
 	);
 	connect(m_modIdResolver.get(), &Flame::FileResolvingTask::failed, [&](QString reason)
 	{
-		m_target->destroyStagingPath(m_stagingPath);
 		m_modIdResolver.reset();
 		emitFailed(tr("Unable to resolve mod IDs:\n") + reason);
 	});
@@ -406,11 +372,5 @@ void InstanceImportTask::processMultiMC()
 			iconList->installIcons({importIconPath});
 		}
 	}
-	if (!m_target->commitStagedInstance(m_stagingPath, m_instName, m_instGroup))
-	{
-		m_target->destroyStagingPath(m_stagingPath);
-		emitFailed(tr("Unable to commit instance"));
-		return;
-	}
 	emitSucceeded();
 }

api/logic/InstanceImportTask.h

@@ -20,7 +20,7 @@ class MULTIMC_LOGIC_EXPORT InstanceImportTask : public Task
 {
 	Q_OBJECT
 public:
-	explicit InstanceImportTask(SettingsObjectPtr settings, const QUrl sourceUrl, BaseInstanceProvider * target, const QString &instName,
+	explicit InstanceImportTask(SettingsObjectPtr settings, const QUrl sourceUrl, const QString & stagingPath, const QString &instName,
 		const QString &instIcon, const QString &instGroup);
 
 protected:
@@ -44,7 +44,6 @@ private: /* data */
 	NetJobPtr m_filesNetJob;
 	shared_qobject_ptr<Flame::FileResolvingTask> m_modIdResolver;
 	QUrl m_sourceUrl;
-	BaseInstanceProvider * m_target;
 	QString m_archivePath;
 	bool m_downloadRequired = false;
 	QString m_instName;