Skip to content
Permalink
Browse files

GH-2591 less std::shared_ptr and more shared_qobject_ptr

This eliminates some weird crashes.
  • Loading branch information...
peterix committed Apr 7, 2019
1 parent 414946c commit 70ed30f9e6b47fbfb3151078ca61a2bb419feccb
@@ -80,7 +80,7 @@ protected slots:
void fileChanged(const QString &path);
void SettingChanged(const Setting & setting, QVariant value);
private:
std::shared_ptr<QFileSystemWatcher> m_watcher;
shared_qobject_ptr<QFileSystemWatcher> m_watcher;
bool is_watching;
QMap<QString, int> name_index;
QVector<MMCIcon> icons;
@@ -175,11 +175,6 @@ QString BaseInstance::instanceRoot() const
return m_rootDir;
}

InstancePtr BaseInstance::getSharedPtr()
{
return shared_from_this();
}

SettingsObjectPtr BaseInstance::settings() const
{
return m_settings;
@@ -253,7 +248,7 @@ QStringList BaseInstance::extraArguments() const
return Commandline::splitArgs(settings()->get("JvmArgs").toString());
}

std::shared_ptr<LaunchTask> BaseInstance::getLaunchTask()
shared_qobject_ptr<LaunchTask> BaseInstance::getLaunchTask()
{
return m_launchProcess;
}
@@ -134,8 +134,6 @@ class MULTIMC_LOGIC_EXPORT BaseInstance : public QObject, public std::enable_sha
/// Sets the last launched time to 'val' milliseconds since epoch
void setLastLaunch(qint64 val = QDateTime::currentMSecsSinceEpoch());

InstancePtr getSharedPtr();

/*!
* \brief Gets this instance's settings object.
* This settings object stores instance-specific settings.
@@ -147,10 +145,10 @@ class MULTIMC_LOGIC_EXPORT BaseInstance : public QObject, public std::enable_sha
virtual shared_qobject_ptr<Task> createUpdateTask(Net::Mode mode) = 0;

/// returns a valid launcher (task container)
virtual std::shared_ptr<LaunchTask> createLaunchTask(AuthSessionPtr account) = 0;
virtual shared_qobject_ptr<LaunchTask> createLaunchTask(AuthSessionPtr account) = 0;

/// returns the current launch task (if any)
std::shared_ptr<LaunchTask> getLaunchTask();
shared_qobject_ptr<LaunchTask> getLaunchTask();

/*!
* Create envrironment variables for running the instance
@@ -241,7 +239,7 @@ class MULTIMC_LOGIC_EXPORT BaseInstance : public QObject, public std::enable_sha
*/
void propertiesChanged(BaseInstance *inst);

void launchTaskChanged(std::shared_ptr<LaunchTask>);
void launchTaskChanged(shared_qobject_ptr<LaunchTask>);

void runningStatusChanged(bool running);

@@ -255,7 +253,7 @@ protected slots:
SettingsObjectPtr m_settings;
// InstanceFlags m_flags;
bool m_isRunning = false;
std::shared_ptr<LaunchTask> m_launchProcess;
shared_qobject_ptr<LaunchTask> m_launchProcess;
QDateTime m_timeStarted;

private: /* data */
@@ -265,6 +263,6 @@ protected slots:
bool m_hasBrokenVersion = false;
};

Q_DECLARE_METATYPE(std::shared_ptr<BaseInstance>)
Q_DECLARE_METATYPE(shared_qobject_ptr<BaseInstance>)
//Q_DECLARE_METATYPE(BaseInstance::InstanceFlag)
//Q_DECLARE_OPERATORS_FOR_FLAGS(BaseInstance::InstanceFlags)
@@ -1,5 +1,6 @@
#pragma once
#include "BaseInstance.h"
#include "launch/LaunchTask.h"

class NullInstance: public BaseInstance
{
@@ -11,46 +12,46 @@ class NullInstance: public BaseInstance
setVersionBroken(true);
}
virtual ~NullInstance() {};
virtual void saveNow() override
void saveNow() override
{
}
virtual QString getStatusbarDescription() override
QString getStatusbarDescription() override
{
return tr("Unknown instance type");
};
virtual QSet< QString > traits() const override
QSet< QString > traits() const override
{
return {};
};
virtual QString instanceConfigFolder() const override
QString instanceConfigFolder() const override
{
return instanceRoot();
};
virtual std::shared_ptr<LaunchTask> createLaunchTask(AuthSessionPtr) override
shared_qobject_ptr<LaunchTask> createLaunchTask(AuthSessionPtr) override
{
return nullptr;
}
virtual shared_qobject_ptr< Task > createUpdateTask(Net::Mode mode) override
shared_qobject_ptr< Task > createUpdateTask(Net::Mode mode) override
{
return nullptr;
}
virtual QProcessEnvironment createEnvironment() override
QProcessEnvironment createEnvironment() override
{
return QProcessEnvironment();
}
virtual QMap<QString, QString> getVariables() const override
QMap<QString, QString> getVariables() const override
{
return QMap<QString, QString>();
}
virtual IPathMatcher::Ptr getLogFileMatcher() override
IPathMatcher::Ptr getLogFileMatcher() override
{
return nullptr;
}
virtual QString getLogFileRoot() override
QString getLogFileRoot() override
{
return instanceRoot();
}
virtual QString typeName() const override
QString typeName() const override
{
return "Null";
}
@@ -75,8 +75,8 @@ void JavaChecker::stderrReady()
void JavaChecker::finished(int exitcode, QProcess::ExitStatus status)
{
killTimer.stop();
QProcessPtr _process;
_process.swap(process);
QProcessPtr _process = process;
process.reset();

JavaCheckResult result;
{
@@ -3,6 +3,8 @@
#include <QTimer>
#include <memory>

#include "QObjectPtr.h"

#include "multimc_logic_export.h"

#include "JavaVersion.h"
@@ -27,8 +29,8 @@ struct MULTIMC_LOGIC_EXPORT JavaCheckResult
} validity = Validity::Errored;
};

typedef std::shared_ptr<QProcess> QProcessPtr;
typedef std::shared_ptr<JavaChecker> JavaCheckerPtr;
typedef shared_qobject_ptr<QProcess> QProcessPtr;
typedef shared_qobject_ptr<JavaChecker> JavaCheckerPtr;
class MULTIMC_LOGIC_EXPORT JavaChecker : public QObject
{
Q_OBJECT
@@ -20,7 +20,7 @@
#include "tasks/Task.h"

class JavaCheckerJob;
typedef std::shared_ptr<JavaCheckerJob> JavaCheckerJobPtr;
typedef shared_qobject_ptr<JavaCheckerJob> JavaCheckerJobPtr;

// FIXME: this just seems horribly redundant
class JavaCheckerJob : public Task
@@ -149,7 +149,7 @@ void JavaListLoadTask::executeTask()
JavaUtils ju;
QList<QString> candidate_paths = ju.FindJavaPaths();

m_job = std::shared_ptr<JavaCheckerJob>(new JavaCheckerJob("Java detection"));
m_job = new JavaCheckerJob("Java detection");
connect(m_job.get(), &Task::finished, this, &JavaListLoadTask::javaCheckerFinished);
connect(m_job.get(), &Task::progress, this, &Task::setProgress);

@@ -24,6 +24,8 @@
#include "JavaCheckerJob.h"
#include "JavaInstall.h"

#include "QObjectPtr.h"

#include "multimc_logic_export.h"

class JavaListLoadTask;
@@ -75,7 +77,7 @@ public slots:
void javaCheckerFinished();

protected:
std::shared_ptr<JavaCheckerJob> m_job;
shared_qobject_ptr<JavaCheckerJob> m_job;
JavaInstallList *m_list;
JavaInstall *m_currentRecommended;
};
@@ -60,7 +60,7 @@ void CheckJava::executeTask()
// if timestamps are not the same, or something is missing, check!
if (javaUnixTime != storedUnixTime || storedVersion.size() == 0 || storedArchitecture.size() == 0)
{
m_JavaChecker = std::make_shared<JavaChecker>();
m_JavaChecker = new JavaChecker();
emit logLine(tr("Checking Java version..."), MessageLevel::MultiMC);
connect(m_JavaChecker.get(), &JavaChecker::checkFinished, this, &CheckJava::checkJavaFinished);
m_JavaChecker->m_path = realJavaPath;
@@ -33,9 +33,9 @@ void LaunchTask::init()
m_instance->setRunning(true);
}

std::shared_ptr<LaunchTask> LaunchTask::create(InstancePtr inst)
shared_qobject_ptr<LaunchTask> LaunchTask::create(InstancePtr inst)
{
std::shared_ptr<LaunchTask> proc(new LaunchTask(inst));
shared_qobject_ptr<LaunchTask> proc(new LaunchTask(inst));
proc->init();
return proc;
}
@@ -44,12 +44,12 @@ LaunchTask::LaunchTask(InstancePtr instance): m_instance(instance)
{
}

void LaunchTask::appendStep(std::shared_ptr<LaunchStep> step)
void LaunchTask::appendStep(shared_qobject_ptr<LaunchStep> step)
{
m_steps.append(step);
}

void LaunchTask::prependStep(std::shared_ptr<LaunchStep> step)
void LaunchTask::prependStep(shared_qobject_ptr<LaunchStep> step)
{
m_steps.prepend(step);
}
@@ -45,11 +45,11 @@ class MULTIMC_LOGIC_EXPORT LaunchTask: public Task
};

public: /* methods */
static std::shared_ptr<LaunchTask> create(InstancePtr inst);
static shared_qobject_ptr<LaunchTask> create(InstancePtr inst);
virtual ~LaunchTask() {};

void appendStep(std::shared_ptr<LaunchStep> step);
void prependStep(std::shared_ptr<LaunchStep> step);
void appendStep(shared_qobject_ptr<LaunchStep> step);
void prependStep(shared_qobject_ptr<LaunchStep> step);
void setCensorFilter(QMap<QString, QString> filter);

InstancePtr instance()
@@ -117,7 +117,7 @@ public slots:
protected: /* data */
InstancePtr m_instance;
shared_qobject_ptr<LogModel> m_logModel;
QList <std::shared_ptr<LaunchStep>> m_steps;
QList <shared_qobject_ptr<LaunchStep>> m_steps;
QMap<QString, QString> m_censorFilter;
int currentStep = -1;
State state = NotStarted;
@@ -777,95 +777,90 @@ shared_qobject_ptr<Task> MinecraftInstance::createUpdateTask(Net::Mode mode)
return nullptr;
}

std::shared_ptr<LaunchTask> MinecraftInstance::createLaunchTask(AuthSessionPtr session)
shared_qobject_ptr<LaunchTask> MinecraftInstance::createLaunchTask(AuthSessionPtr session)
{
auto process = LaunchTask::create(std::dynamic_pointer_cast<MinecraftInstance>(getSharedPtr()));
// FIXME: get rid of shared_from_this ...
auto process = LaunchTask::create(std::dynamic_pointer_cast<MinecraftInstance>(shared_from_this()));
auto pptr = process.get();

ENV.icons()->saveIcon(iconKey(), FS::PathCombine(gameRoot(), "icon.png"), "PNG");

// print a header
{
process->appendStep(std::make_shared<TextPrint>(pptr, "Minecraft folder is:\n" + gameRoot() + "\n\n", MessageLevel::MultiMC));
process->appendStep(new TextPrint(pptr, "Minecraft folder is:\n" + gameRoot() + "\n\n", MessageLevel::MultiMC));
}

// check java
{
auto step = std::make_shared<CheckJava>(pptr);
process->appendStep(step);
process->appendStep(new CheckJava(pptr));
}

// check launch method
QStringList validMethods = {"LauncherPart", "DirectJava"};
QString method = launchMethod();
if(!validMethods.contains(method))
{
process->appendStep(std::make_shared<TextPrint>(pptr, "Selected launch method \"" + method + "\" is not valid.\n", MessageLevel::Fatal));
process->appendStep(new TextPrint(pptr, "Selected launch method \"" + method + "\" is not valid.\n", MessageLevel::Fatal));
return process;
}

// run pre-launch command if that's needed
if(getPreLaunchCommand().size())
{
auto step = std::make_shared<PreLaunchCommand>(pptr);
auto step = new PreLaunchCommand(pptr);
step->setWorkingDirectory(gameRoot());
process->appendStep(step);
}

// if we aren't in offline mode,.
if(session->status != AuthSession::PlayableOffline)
{
process->appendStep(std::make_shared<ClaimAccount>(pptr, session));
process->appendStep(std::make_shared<Update>(pptr, Net::Mode::Online));
process->appendStep(new ClaimAccount(pptr, session));
process->appendStep(new Update(pptr, Net::Mode::Online));
}
else
{
process->appendStep(std::make_shared<Update>(pptr, Net::Mode::Offline));
process->appendStep(new Update(pptr, Net::Mode::Offline));
}

// if there are any jar mods
{
auto step = std::make_shared<ModMinecraftJar>(pptr);
process->appendStep(step);
process->appendStep(new ModMinecraftJar(pptr));
}

// print some instance info here...
{
auto step = std::make_shared<PrintInstanceInfo>(pptr, session);
process->appendStep(step);
process->appendStep(new PrintInstanceInfo(pptr, session));
}

// create the server-resource-packs folder (workaround for Minecraft bug MCL-3732)
{
auto step = std::make_shared<CreateServerResourcePacksFolder>(pptr);
process->appendStep(step);
process->appendStep(new CreateServerResourcePacksFolder(pptr));
}

// extract native jars if needed
{
auto step = std::make_shared<ExtractNatives>(pptr);
process->appendStep(step);
process->appendStep(new ExtractNatives(pptr));
}

// reconstruct assets if needed
{
auto step = std::make_shared<ReconstructAssets>(pptr);
process->appendStep(step);
process->appendStep(new ReconstructAssets(pptr));
}

{
// actually launch the game
auto method = launchMethod();
if(method == "LauncherPart")
{
auto step = std::make_shared<LauncherPartLaunch>(pptr);
auto step = new LauncherPartLaunch(pptr);
step->setWorkingDirectory(gameRoot());
step->setAuthSession(session);
process->appendStep(step);
}
else if (method == "DirectJava")
{
auto step = std::make_shared<DirectJavaLaunch>(pptr);
auto step = new DirectJavaLaunch(pptr);
step->setWorkingDirectory(gameRoot());
step->setAuthSession(session);
process->appendStep(step);
@@ -875,7 +870,7 @@ std::shared_ptr<LaunchTask> MinecraftInstance::createLaunchTask(AuthSessionPtr s
// run post-exit command if that's needed
if(getPostExitCommand().size())
{
auto step = std::make_shared<PostLaunchCommand>(pptr);
auto step = new PostLaunchCommand(pptr);
step->setWorkingDirectory(gameRoot());
process->appendStep(step);
}
Oops, something went wrong.

0 comments on commit 70ed30f

Please sign in to comment.
You can’t perform that action at this time.