NOISSUE Refactor and sanitize MultiMC startup/shutdown

* Always create main window.
* Properly handle netowrk manager - it was created twice, leading to potential crashes.

api/logic/Env.cpp

@@ -10,20 +10,30 @@
 #include "wonko/WonkoIndex.h"
 #include <QDebug>
 
+
+class Env::Private
+{
+public:
+	QNetworkAccessManager m_qnam;
+	shared_qobject_ptr<HttpMetaCache> m_metacache;
+	std::shared_ptr<IIconList> m_iconlist;
+	QMap<QString, std::shared_ptr<BaseVersionList>> m_versionLists;
+	shared_qobject_ptr<WonkoIndex> m_wonkoIndex;
+	QString m_wonkoRootUrl;
+};
+
 /*
  * The *NEW* global rat nest of an object. Handle with care.
  */
 
 Env::Env()
 {
-	m_qnam = std::make_shared<QNetworkAccessManager>();
+	d = new Private();
 }
 
-void Env::destroy()
+Env::~Env()
 {
-	m_metacache.reset();
+	delete d;
-	m_qnam.reset();
-	m_versionLists.clear();
 }
 
 Env& Env::Env::getInstance()
@@ -32,85 +42,26 @@ Env& Env::Env::getInstance()
 	return instance;
 }
 
-std::shared_ptr< HttpMetaCache > Env::metacache()
+shared_qobject_ptr< HttpMetaCache > Env::metacache()
 {
-	Q_ASSERT(m_metacache != nullptr);
+	return d->m_metacache;
-	return m_metacache;
 }
 
-std::shared_ptr< QNetworkAccessManager > Env::qnam()
+QNetworkAccessManager& Env::qnam() const
 {
-	return m_qnam;
+	return d->m_qnam;
 }
 
 std::shared_ptr<IIconList> Env::icons()
 {
-	return m_iconlist;
+	return d->m_iconlist;
 }
 
 void Env::registerIconList(std::shared_ptr<IIconList> iconlist)
 {
-	m_iconlist = iconlist;
+	d->m_iconlist = iconlist;
 }
 
-/*
-class NullVersion : public BaseVersion
-{
-	Q_OBJECT
-public:
-	virtual QString name()
-	{
-		return "null";
-	}
-	virtual QString descriptor()
-	{
-		return "null";
-	}
-	virtual QString typeString() const
-	{
-		return "Null";
-	}
-};
-
-class NullTask: public Task
-{
-	Q_OBJECT
-public:
-	virtual void executeTask()
-	{
-		emitFailed(tr("Nothing to do."));
-	}
-};
-
-class NullVersionList: public BaseVersionList
-{
-	Q_OBJECT
-public:
-	virtual const BaseVersionPtr at(int i) const
-	{
-		return std::make_shared<NullVersion>();
-	}
-	virtual int count() const
-	{
-		return 0;
-	};
-	virtual Task* getLoadTask()
-	{
-		return new NullTask;
-	}
-	virtual bool isLoaded()
-	{
-		return false;
-	}
-	virtual void sort()
-	{
-	}
-	virtual void updateListData(QList< BaseVersionPtr >)
-	{
-	}
-};
-*/
-
 BaseVersionPtr Env::getVersion(QString component, QString version)
 {
 	auto list = getVersionList(component);
@@ -123,8 +74,8 @@ BaseVersionPtr Env::getVersion(QString component, QString version)
 
 std::shared_ptr< BaseVersionList > Env::getVersionList(QString component)
 {
-	auto iter = m_versionLists.find(component);
+	auto iter = d->m_versionLists.find(component);
-	if(iter != m_versionLists.end())
+	if(iter != d->m_versionLists.end())
 	{
 		return *iter;
 	}
@@ -134,21 +85,22 @@ std::shared_ptr< BaseVersionList > Env::getVersionList(QString component)
 
 void Env::registerVersionList(QString name, std::shared_ptr< BaseVersionList > vlist)
 {
-	m_versionLists[name] = vlist;
+	d->m_versionLists[name] = vlist;
 }
 
-std::shared_ptr<WonkoIndex> Env::wonkoIndex()
+shared_qobject_ptr<WonkoIndex> Env::wonkoIndex()
 {
-	if (!m_wonkoIndex)
+	if (!d->m_wonkoIndex)
 	{
-		m_wonkoIndex = std::make_shared<WonkoIndex>();
+		d->m_wonkoIndex.reset(new WonkoIndex());
 	}
-	return m_wonkoIndex;
+	return d->m_wonkoIndex;
 }
 
 
 void Env::initHttpMetaCache()
 {
+	auto &m_metacache = d->m_metacache;
 	m_metacache.reset(new HttpMetaCache("metacache"));
 	m_metacache->addBase("asset_indexes", QDir("assets/indexes").absolutePath());
 	m_metacache->addBase("asset_objects", QDir("assets/objects").absolutePath());
@@ -192,8 +144,7 @@ void Env::updateProxySettings(QString proxyTypeStr, QString addr, int port, QStr
 
 	qDebug() << "Detecting proxy settings...";
 	QNetworkProxy proxy = QNetworkProxy::applicationProxy();
-	if (m_qnam.get())
+	d->m_qnam.setProxy(proxy);
-		m_qnam->setProxy(proxy);
 	QString proxyDesc;
 	if (proxy.type() == QNetworkProxy::NoProxy)
 	{
@@ -229,4 +180,14 @@ void Env::updateProxySettings(QString proxyTypeStr, QString addr, int port, QStr
 	qDebug() << proxyDesc;
 }
 
+QString Env::wonkoRootUrl() const
+{
+	return d->m_wonkoRootUrl;
+}
+
+void Env::setWonkoRootUrl(const QString& url)
+{
+	d->m_wonkoRootUrl = url;
+}
+
 #include "Env.moc"

api/logic/Env.h

@@ -7,6 +7,8 @@
 
 #include "multimc_logic_export.h"
 
+#include "QObjectPtr.h"
+
 class QNetworkAccessManager;
 class HttpMetaCache;
 class BaseVersionList;
@@ -22,16 +24,15 @@ class MULTIMC_LOGIC_EXPORT Env
 {
 	friend class MultiMC;
 private:
+	class Private;
 	Env();
+	~Env();
 public:
 	static Env& getInstance();
 
-	// call when Qt stuff is being torn down
+	QNetworkAccessManager &qnam() const;
-	void destroy();
 
-	std::shared_ptr<QNetworkAccessManager> qnam();
+	shared_qobject_ptr<HttpMetaCache> metacache();
-
-	std::shared_ptr<HttpMetaCache> metacache();
 
 	std::shared_ptr<IIconList> icons();
 
@@ -51,16 +52,11 @@ public:
 
 	void registerIconList(std::shared_ptr<IIconList> iconlist);
 
-	std::shared_ptr<WonkoIndex> wonkoIndex();
+	shared_qobject_ptr<WonkoIndex> wonkoIndex();
 
-	QString wonkoRootUrl() const { return m_wonkoRootUrl; }
+	QString wonkoRootUrl() const;
-	void setWonkoRootUrl(const QString &url) { m_wonkoRootUrl = url; }
+	void setWonkoRootUrl(const QString &url);
 
 protected:
-	std::shared_ptr<QNetworkAccessManager> m_qnam;
+	Private * d;
-	std::shared_ptr<HttpMetaCache> m_metacache;
-	std::shared_ptr<IIconList> m_iconlist;
-	QMap<QString, std::shared_ptr<BaseVersionList>> m_versionLists;
-	std::shared_ptr<WonkoIndex> m_wonkoIndex;
-	QString m_wonkoRootUrl;
 };

api/logic/minecraft/MinecraftVersionList.cpp

@@ -425,8 +425,7 @@ MCVListLoadTask::MCVListLoadTask(MinecraftVersionList *vlist)
 void MCVListLoadTask::executeTask()
 {
 	setStatus(tr("Loading instance version list..."));
-	auto worker = ENV.qnam();
+	vlistReply = ENV.qnam().get(QNetworkRequest(QUrl("https://launchermeta.mojang.com/mc/game/version_manifest.json")));
-	vlistReply = worker->get(QNetworkRequest(QUrl("https://launchermeta.mojang.com/mc/game/version_manifest.json")));
 	connect(vlistReply, SIGNAL(finished()), this, SLOT(list_downloaded()));
 }
 

api/logic/minecraft/SkinUpload.cpp

@@ -40,7 +40,7 @@ void SkinUpload::executeTask()
 	multiPart->append(model);
 	multiPart->append(skin);
 
-	QNetworkReply *rep = ENV.qnam()->put(request, multiPart);
+	QNetworkReply *rep = ENV.qnam().put(request, multiPart);
 	m_reply = std::shared_ptr<QNetworkReply>(rep);
 
 	setStatus(tr("Uploading skin"));

api/logic/minecraft/auth/YggdrasilTask.cpp

@@ -42,13 +42,12 @@ void YggdrasilTask::executeTask()
 	// Get the content of the request we're going to send to the server.
 	QJsonDocument doc(getRequestContent());
 
-	auto worker = ENV.qnam();
 	QUrl reqUrl("https://" + URLConstants::AUTH_BASE + getEndpoint());
 	QNetworkRequest netRequest(reqUrl);
 	netRequest.setHeader(QNetworkRequest::ContentTypeHeader, "application/json");
 
 	QByteArray requestData = doc.toJson();
-	m_netReply = worker->post(netRequest, requestData);
+	m_netReply = ENV.qnam().post(netRequest, requestData);
 	connect(m_netReply, &QNetworkReply::finished, this, &YggdrasilTask::processReply);
 	connect(m_netReply, &QNetworkReply::uploadProgress, this, &YggdrasilTask::refreshTimers);
 	connect(m_netReply, &QNetworkReply::downloadProgress, this, &YggdrasilTask::refreshTimers);

api/logic/minecraft/forge/ForgeXzDownload.cpp

@@ -61,15 +61,12 @@ void ForgeXzDownload::start()
 	request.setRawHeader(QString("If-None-Match").toLatin1(), m_entry->getETag().toLatin1());
 	request.setHeader(QNetworkRequest::UserAgentHeader, "MultiMC/5.0 (Cached)");
 
-	auto worker = ENV.qnam();
+	QNetworkReply *rep = ENV.qnam().get(request);
-	QNetworkReply *rep = worker->get(request);
 
 	m_reply.reset(rep);
-	connect(rep, SIGNAL(downloadProgress(qint64, qint64)),
+	connect(rep, SIGNAL(downloadProgress(qint64, qint64)), SLOT(downloadProgress(qint64, qint64)));
-			SLOT(downloadProgress(qint64, qint64)));
 	connect(rep, SIGNAL(finished()), SLOT(downloadFinished()));
-	connect(rep, SIGNAL(error(QNetworkReply::NetworkError)),
+	connect(rep, SIGNAL(error(QNetworkReply::NetworkError)), SLOT(downloadError(QNetworkReply::NetworkError)));
-			SLOT(downloadError(QNetworkReply::NetworkError)));
 	connect(rep, SIGNAL(readyRead()), SLOT(downloadReadyRead()));
 }
 

api/logic/minecraft/legacy/LegacyUpdate.cpp

@@ -195,7 +195,7 @@ void LegacyUpdate::lwjglStart()
 	QString url = version->url();
 	QUrl realUrl(url);
 	QString hostname = realUrl.host();
-	auto worker = ENV.qnam();
+	auto worker = &ENV.qnam();
 	QNetworkRequest req(realUrl);
 	req.setRawHeader("Host", hostname.toLatin1());
 	req.setHeader(QNetworkRequest::UserAgentHeader, "MultiMC/5.0 (Cached)");
@@ -203,7 +203,7 @@ void LegacyUpdate::lwjglStart()
 
 	m_reply = std::shared_ptr<QNetworkReply>(rep);
 	connect(rep, &QNetworkReply::downloadProgress, this, &LegacyUpdate::progress);
-	connect(worker.get(), &QNetworkAccessManager::finished, this, &LegacyUpdate::lwjglFinished);
+	connect(worker, &QNetworkAccessManager::finished, this, &LegacyUpdate::lwjglFinished);
 }
 
 void LegacyUpdate::lwjglFinished(QNetworkReply *reply)
@@ -219,7 +219,7 @@ void LegacyUpdate::lwjglFinished(QNetworkReply *reply)
 				   "a row. YMMV");
 		return;
 	}
-	auto worker = ENV.qnam();
+	auto worker = &ENV.qnam();
 	// Here i check if there is a cookie for me in the reply and extract it
 	QList<QNetworkCookie> cookies =
 		qvariant_cast<QList<QNetworkCookie>>(reply->header(QNetworkRequest::SetCookieHeader));

api/logic/minecraft/legacy/LwjglVersionList.cpp

@@ -82,11 +82,10 @@ void LWJGLVersionList::loadList()
 	Q_ASSERT_X(!m_loading, "loadList", "list is already loading (m_loading is true)");
 
 	setLoading(true);
-	auto worker = ENV.qnam();
 	QNetworkRequest req(QUrl(RSS_URL));
 	req.setRawHeader("Accept", "application/rss+xml, text/xml, */*");
 	req.setRawHeader("User-Agent", "MultiMC/5.0 (Uncached)");
-	reply = worker->get(req);
+	reply = ENV.qnam().get(req);
 	connect(reply, SIGNAL(finished()), SLOT(netRequestComplete()));
 }
 

api/logic/net/Download.cpp

@@ -95,8 +95,7 @@ void Download::start()
 
 	request.setHeader(QNetworkRequest::UserAgentHeader, "MultiMC/5.0");
 
-	auto worker = ENV.qnam();
+	QNetworkReply *rep =  ENV.qnam().get(request);
-	QNetworkReply *rep = worker->get(request);
 
 	m_reply.reset(rep);
 	connect(rep, SIGNAL(downloadProgress(qint64, qint64)), SLOT(downloadProgress(qint64, qint64)));

api/logic/net/PasteUpload.cpp

@@ -36,8 +36,7 @@ void PasteUpload::executeTask()
 	request.setRawHeader("Content-Type", "application/x-www-form-urlencoded");
 	request.setRawHeader("Content-Length", QByteArray::number(m_text.size()));
 
-	auto worker = ENV.qnam();
+	QNetworkReply *rep = ENV.qnam().post(request, buf);
-	QNetworkReply *rep = worker->post(request, buf);
 
 	m_reply = std::shared_ptr<QNetworkReply>(rep);
 	setStatus(tr("Uploading to paste.ee"));

api/logic/screenshots/ImgurAlbumCreation.cpp

@@ -33,14 +33,12 @@ void ImgurAlbumCreation::start()
 
 	const QByteArray data = "ids=" + ids.join(',').toUtf8() + "&title=Minecraft%20Screenshots&privacy=hidden";
 
-	auto worker = ENV.qnam();
+	QNetworkReply *rep = ENV.qnam().post(request, data);
-	QNetworkReply *rep = worker->post(request, data);
 
 	m_reply.reset(rep);
 	connect(rep, &QNetworkReply::uploadProgress, this, &ImgurAlbumCreation::downloadProgress);
 	connect(rep, &QNetworkReply::finished, this, &ImgurAlbumCreation::downloadFinished);
-	connect(rep, SIGNAL(error(QNetworkReply::NetworkError)),
+	connect(rep, SIGNAL(error(QNetworkReply::NetworkError)), SLOT(downloadError(QNetworkReply::NetworkError)));
-			SLOT(downloadError(QNetworkReply::NetworkError)));
 }
 void ImgurAlbumCreation::downloadError(QNetworkReply::NetworkError error)
 {

api/logic/screenshots/ImgurUpload.cpp

@@ -49,8 +49,7 @@ void ImgurUpload::start()
 	namePart.setBody(m_shot->m_file.baseName().toUtf8());
 	multipart->append(namePart);
 
-	auto worker = ENV.qnam();
+	QNetworkReply *rep = ENV.qnam().post(request, multipart);
-	QNetworkReply *rep = worker->post(request, multipart);
 
 	m_reply.reset(rep);
 	connect(rep, &QNetworkReply::uploadProgress, this, &ImgurUpload::downloadProgress);

api/logic/wonko/WonkoIndex_test.cpp

@@ -12,7 +12,7 @@ private
 slots:
 	void test_isProvidedByEnv()
 	{
-		QVERIFY(ENV.wonkoIndex() != nullptr);
+		QVERIFY(ENV.wonkoIndex());
 		QCOMPARE(ENV.wonkoIndex(), ENV.wonkoIndex());
 	}
 

application/MultiMC.cpp

@@ -286,12 +286,17 @@ MultiMC::MultiMC(int &argc, char **argv) : QApplication(argc, argv)
 		auto inst = instances()->getInstanceById(m_instanceIdToLaunch);
 		if(inst)
 		{
-			minecraftlist();
+			// minimized main window
+			showMainWindow(true);
 			launch(inst, true, nullptr);
 			return;
 		}
 	}
-	showMainWindow();
+	if(!m_mainWindow)
+	{
+		// normal main window
+		showMainWindow(false);
+	}
 }
 
 MultiMC::~MultiMC()
@@ -340,9 +345,6 @@ void MultiMC::initNetwork()
 	// init the http meta cache
 	ENV.initHttpMetaCache();
 
-	// create the global network manager
-	ENV.m_qnam.reset(new QNetworkAccessManager(this));
-
 	// init proxy settings
 	{
 		QString proxyTypeStr = settings()->get("ProxyType").toString();
@@ -1052,7 +1054,6 @@ void MultiMC::onExit()
 	{
 		// m_instances->saveGroupList();
 	}
-	ENV.destroy();
 	if(logFile)
 	{
 		logFile->flush();
@@ -1126,6 +1127,12 @@ void MultiMC::controllerSucceeded()
 		}
 	}
 	extras.controller.reset();
+	// quit when there are no more windows.
+	if(m_openWindows == 0)
+	{
+		m_status = Status::Succeeded;
+		quit();
+	}
 }
 
 void MultiMC::controllerFailed(const QString& error)
@@ -1139,9 +1146,15 @@ void MultiMC::controllerFailed(const QString& error)
 
 	// on failure, do... nothing
 	extras.controller.reset();
+	// quit when there are no more windows.
+	if(m_openWindows == 0)
+	{
+		m_status = Status::Failed;
+		quit();
+	}
 }
 
-MainWindow * MultiMC::showMainWindow()
+MainWindow* MultiMC::showMainWindow(bool minimized)
 {
 	if(m_mainWindow)
 	{
@@ -1154,9 +1167,18 @@ MainWindow * MultiMC::showMainWindow()
 		m_mainWindow = new MainWindow();
 		m_mainWindow->restoreState(QByteArray::fromBase64(MMC->settings()->get("MainWindowState").toByteArray()));
 		m_mainWindow->restoreGeometry(QByteArray::fromBase64(MMC->settings()->get("MainWindowGeometry").toByteArray()));
+		if(minimized)
+		{
+			m_mainWindow->showMinimized();
+		}
+		else
+		{
 			m_mainWindow->show();
+		}
+
 		m_mainWindow->checkSetDefaultJava();
 		m_mainWindow->checkInstancePathForProblems();
+		m_openWindows++;
 	}
 	return m_mainWindow;
 }
@@ -1177,13 +1199,13 @@ InstanceWindow *MultiMC::showInstanceWindow(InstancePtr instance, QString page)
 	else
 	{
 		window = new InstanceWindow(instance);
+		m_openWindows ++;
 		connect(window, &InstanceWindow::isClosing, this, &MultiMC::on_windowClose);
 	}
 	if(!page.isEmpty())
 	{
 		window->selectPage(page);
 	}
-	m_openWindows ++;
 	if(extras.controller)
 	{
 		extras.controller->setParentWidget(window);

application/MultiMC.h

@@ -142,7 +142,9 @@ public:
 	bool openJsonEditor(const QString &filename);
 
 	InstanceWindow *showInstanceWindow(InstancePtr instance, QString page = QString());
-	MainWindow *showMainWindow();
+	MainWindow *showMainWindow(bool minimized = false);
+
+public slots:
 	void launch(InstancePtr instance, bool online = true, BaseProfilerFactory *profiler = nullptr);
 
 private slots:

application/WonkoGui.h

@@ -1,11 +1,12 @@
 #pragma once
 
 #include <memory>
+#include "QObjectPtr.h"
 
 class QWidget;
 class QString;
 
-using WonkoIndexPtr = std::shared_ptr<class WonkoIndex>;
+using WonkoIndexPtr = shared_qobject_ptr<class WonkoIndex>;
 using WonkoVersionListPtr = std::shared_ptr<class WonkoVersionList>;
 using WonkoVersionPtr = std::shared_ptr<class WonkoVersion>;
 

application/main.cpp

@@ -9,6 +9,10 @@ int main(int argc, char *argv[])
 	// initialize Qt
 	MultiMC app(argc, argv);
 
+	switch (app.status())
+	{
+	case MultiMC::Initialized:
+	{
 		Q_INIT_RESOURCE(instances);
 		Q_INIT_RESOURCE(multimc);
 		Q_INIT_RESOURCE(backgrounds);
@@ -20,11 +24,8 @@ int main(int argc, char *argv[])
 		Q_INIT_RESOURCE(pe_colored);
 		Q_INIT_RESOURCE(OSX);
 		Q_INIT_RESOURCE(iOS);
-
-	switch (app.status())
-	{
-	case MultiMC::Initialized:
 		return app.exec();
+	}
 	case MultiMC::Failed:
 		return 1;
 	case MultiMC::Succeeded: