From 1b884d0a9dc28d8bca38fe8756482d991d0ea850 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Mr=C3=A1zek?= Date: Mon, 4 May 2015 01:20:48 +0200 Subject: GH-907 improve Java testing and PermGen deprecation handling --- logic/CMakeLists.txt | 2 + logic/java/JavaChecker.cpp | 69 +++++++++++++++++++++++++--------- logic/java/JavaChecker.h | 13 ++++++- logic/java/JavaVersionList.cpp | 4 +- logic/minecraft/MinecraftInstance.cpp | 6 ++- logic/minecraft/MinecraftProcess.cpp | 70 +++++++++++++++++++++++++++++++---- logic/settings/PassthroughSetting.cpp | 66 +++++++++++++++++++++++++++++++++ logic/settings/PassthroughSetting.h | 45 ++++++++++++++++++++++ logic/settings/SettingsObject.cpp | 17 +++++++++ logic/settings/SettingsObject.h | 10 +++++ 10 files changed, 273 insertions(+), 29 deletions(-) create mode 100644 logic/settings/PassthroughSetting.cpp create mode 100644 logic/settings/PassthroughSetting.h (limited to 'logic') diff --git a/logic/CMakeLists.txt b/logic/CMakeLists.txt index f22078c1..de1940ad 100644 --- a/logic/CMakeLists.txt +++ b/logic/CMakeLists.txt @@ -191,6 +191,8 @@ SET(LOGIC_SOURCES settings/INISettingsObject.h settings/OverrideSetting.cpp settings/OverrideSetting.h + settings/PassthroughSetting.cpp + settings/PassthroughSetting.h settings/Setting.cpp settings/Setting.h settings/SettingsObject.cpp diff --git a/logic/java/JavaChecker.cpp b/logic/java/JavaChecker.cpp index b62ddfbe..18877898 100644 --- a/logic/java/JavaChecker.cpp +++ b/logic/java/JavaChecker.cpp @@ -1,5 +1,6 @@ #include "JavaChecker.h" #include +#include #include #include #include @@ -15,26 +16,59 @@ void JavaChecker::performCheck() { QString checkerJar = PathCombine(QCoreApplication::applicationDirPath(), "jars", "JavaCheck.jar"); - QStringList args = {"-jar", checkerJar}; + QStringList args; process.reset(new QProcess()); + if(m_args.size()) + { + auto extraArgs = Util::Commandline::splitArgs(m_args); + args.append(extraArgs); + } + if(m_minMem != 0) + { + args << QString("-Xms%1m").arg(m_minMem); + } + if(m_maxMem != 0) + { + args << QString("-Xmx%1m").arg(m_maxMem); + } + if(m_permGen != 64) + { + args << QString("-XX:PermSize=%1m").arg(m_permGen); + } + + args.append({"-jar", checkerJar}); process->setArguments(args); - process->setProgram(path); + process->setProgram(m_path); process->setProcessChannelMode(QProcess::SeparateChannels); - qDebug() << "Running java checker!"; - qDebug() << "Java: " + path; - qDebug() << "Args: {" + args.join("|") + "}"; - - connect(process.get(), SIGNAL(finished(int, QProcess::ExitStatus)), this, - SLOT(finished(int, QProcess::ExitStatus))); - connect(process.get(), SIGNAL(error(QProcess::ProcessError)), this, - SLOT(error(QProcess::ProcessError))); + qDebug() << "Running java checker: " + m_path + args.join(" ");; + + connect(process.get(), SIGNAL(finished(int, QProcess::ExitStatus)), this, SLOT(finished(int, QProcess::ExitStatus))); + connect(process.get(), SIGNAL(error(QProcess::ProcessError)), this, SLOT(error(QProcess::ProcessError))); + connect(process.get(), SIGNAL(readyReadStandardOutput()), this, SLOT(stdoutReady())); + connect(process.get(), SIGNAL(readyReadStandardError()), this, SLOT(stderrReady())); connect(&killTimer, SIGNAL(timeout()), SLOT(timeout())); killTimer.setSingleShot(true); killTimer.start(15000); process->start(); } +void JavaChecker::stdoutReady() +{ + QByteArray data = process->readAllStandardOutput(); + QString added = QString::fromLocal8Bit(data); + added.remove('\r'); + m_stdout += added; +} + +void JavaChecker::stderrReady() +{ + QByteArray data = process->readAllStandardError(); + QString added = QString::fromLocal8Bit(data); + added.remove('\r'); + m_stderr += added; +} + void JavaChecker::finished(int exitcode, QProcess::ExitStatus status) { killTimer.stop(); @@ -43,9 +77,12 @@ void JavaChecker::finished(int exitcode, QProcess::ExitStatus status) JavaCheckResult result; { - result.path = path; - result.id = id; + result.path = m_path; + result.id = m_id; } + result.stderr = m_stderr; + qDebug() << "STDOUT" << m_stdout; + qWarning() << "STDERR" << m_stderr; qDebug() << "Java checker finished with status " << status << " exit code " << exitcode; if (status == QProcess::CrashExit || exitcode == 1) @@ -56,11 +93,9 @@ void JavaChecker::finished(int exitcode, QProcess::ExitStatus status) } bool success = true; - QString p_stdout = _process->readAllStandardOutput(); - qDebug() << p_stdout; QMap results; - QStringList lines = p_stdout.split("\n", QString::SkipEmptyParts); + QStringList lines = m_stdout.split("\n", QString::SkipEmptyParts); for(QString line : lines) { line = line.trimmed(); @@ -105,8 +140,8 @@ void JavaChecker::error(QProcess::ProcessError err) qDebug() << "Java checker has failed to start."; JavaCheckResult result; { - result.path = path; - result.id = id; + result.path = m_path; + result.id = m_id; } emit checkFinished(result); diff --git a/logic/java/JavaChecker.h b/logic/java/JavaChecker.h index e19895f7..29598aa4 100644 --- a/logic/java/JavaChecker.h +++ b/logic/java/JavaChecker.h @@ -12,6 +12,7 @@ struct JavaCheckResult QString mojangPlatform; QString realPlatform; QString javaVersion; + QString stderr; bool valid = false; bool is_64bit = false; int id; @@ -26,17 +27,25 @@ public: explicit JavaChecker(QObject *parent = 0); void performCheck(); - QString path; - int id; + QString m_path; + QString m_args; + int m_id = 0; + int m_minMem = 0; + int m_maxMem = 0; + int m_permGen = 64; signals: void checkFinished(JavaCheckResult result); private: QProcessPtr process; QTimer killTimer; + QString m_stdout; + QString m_stderr; public slots: void timeout(); void finished(int exitcode, QProcess::ExitStatus); void error(QProcess::ProcessError); + void stdoutReady(); + void stderrReady(); }; diff --git a/logic/java/JavaVersionList.cpp b/logic/java/JavaVersionList.cpp index 7889853b..52f2e62f 100644 --- a/logic/java/JavaVersionList.cpp +++ b/logic/java/JavaVersionList.cpp @@ -171,8 +171,8 @@ void JavaListLoadTask::executeTask() qDebug() << " " << candidate; auto candidate_checker = new JavaChecker(); - candidate_checker->path = candidate; - candidate_checker->id = id; + candidate_checker->m_path = candidate; + candidate_checker->m_id = id; m_job->addJavaCheckerAction(JavaCheckerPtr(candidate_checker)); id++; diff --git a/logic/minecraft/MinecraftInstance.cpp b/logic/minecraft/MinecraftInstance.cpp index e4273cb9..2813a609 100644 --- a/logic/minecraft/MinecraftInstance.cpp +++ b/logic/minecraft/MinecraftInstance.cpp @@ -9,11 +9,15 @@ MinecraftInstance::MinecraftInstance(SettingsObjectPtr globalSettings, SettingsO { // Java Settings m_settings->registerSetting("OverrideJava", false); - m_settings->registerSetting("OverrideJavaLocation", false); + auto locationOverride = m_settings->registerSetting("OverrideJavaLocation", false); m_settings->registerSetting("OverrideJavaArgs", false); m_settings->registerOverride(globalSettings->getSetting("JavaPath")); m_settings->registerOverride(globalSettings->getSetting("JvmArgs")); + // special! + m_settings->registerPassthrough(globalSettings->getSetting("JavaTimestamp"), locationOverride); + m_settings->registerPassthrough(globalSettings->getSetting("JavaVersion"), locationOverride); + // Window Size m_settings->registerSetting("OverrideWindow", false); m_settings->registerOverride(globalSettings->getSetting("LaunchMaximized")); diff --git a/logic/minecraft/MinecraftProcess.cpp b/logic/minecraft/MinecraftProcess.cpp index 9d18ad90..10d30b73 100644 --- a/logic/minecraft/MinecraftProcess.cpp +++ b/logic/minecraft/MinecraftProcess.cpp @@ -16,6 +16,8 @@ */ #include "minecraft/MinecraftProcess.h" #include "BaseInstance.h" +#include +#include #include #include @@ -141,11 +143,18 @@ QStringList MinecraftProcess::javaArguments() const args << QString("-Xms%1m").arg(m_instance->settings().get("MinMemAlloc").toInt()); args << QString("-Xmx%1m").arg(m_instance->settings().get("MaxMemAlloc").toInt()); - auto permgen = m_instance->settings().get("PermGen").toInt(); - if (permgen != 64) + + // No PermGen in newer java. + auto javaVersion = m_instance->settings().get("JavaVersion"); + if(Strings::naturalCompare(javaVersion.toString(), "1.8.0", Qt::CaseInsensitive) < 0) { - args << QString("-XX:PermSize=%1m").arg(permgen); + auto permgen = m_instance->settings().get("PermGen").toInt(); + if (permgen != 64) + { + args << QString("-XX:PermSize=%1m").arg(permgen); + } } + args << "-Duser.language=en"; if (!m_nativeFolder.isEmpty()) args << QString("-Djava.library.path=%1").arg(m_nativeFolder); @@ -167,12 +176,8 @@ void MinecraftProcess::arm() m_instance->setLastLaunch(); - QStringList args = javaArguments(); - QString JavaPath = m_instance->settings().get("JavaPath").toString(); emit log("Java path is:\n" + JavaPath + "\n\n"); - QString allArgs = args.join(", "); - emit log("Java Arguments:\n[" + censorPrivateInfo(allArgs) + "]\n\n"); auto realJavaPath = QStandardPaths::findExecutable(JavaPath); if (realJavaPath.isEmpty()) @@ -182,6 +187,57 @@ void MinecraftProcess::arm() MessageLevel::Warning); } + // check java version here. + { + QFileInfo javaInfo(realJavaPath); + qlonglong javaUnixTime = javaInfo.lastModified().toMSecsSinceEpoch(); + auto storedUnixTime = m_instance->settings().get("JavaTimestamp").toLongLong(); + // if they are not the same, check! + if(javaUnixTime != storedUnixTime) + { + QEventLoop ev; + auto checker = std::make_shared(); + bool successful = false; + QString errorLog; + QString version; + emit log(tr("Checking Java version..."), MessageLevel::MultiMC); + connect(checker.get(), &JavaChecker::checkFinished, + [&](JavaCheckResult result) + { + successful = result.valid; + errorLog = result.stderr; + version = result.javaVersion; + ev.exit(); + }); + checker->m_path = realJavaPath; + checker->performCheck(); + ev.exec(); + if(!successful) + { + // Error message displayed if java can't start + emit log(tr("Could not start java:"), MessageLevel::Error); + auto lines = errorLog.split('\n'); + for(auto line: lines) + { + emit log(line, MessageLevel::Error); + } + emit log("\nCheck your MultiMC Java settings.", MessageLevel::MultiMC); + m_instance->cleanupAfterRun(); + emit launch_failed(m_instance); + // not running, failed + m_instance->setRunning(false); + return; + } + emit log(tr("Java version is %1!\n").arg(version), MessageLevel::MultiMC); + m_instance->settings().set("JavaVersion", version); + m_instance->settings().set("JavaTimestamp", javaUnixTime); + } + } + + QStringList args = javaArguments(); + QString allArgs = args.join(", "); + emit log("Java Arguments:\n[" + censorPrivateInfo(allArgs) + "]\n\n"); + // instantiate the launcher part start(JavaPath, args); if (!waitForStarted()) diff --git a/logic/settings/PassthroughSetting.cpp b/logic/settings/PassthroughSetting.cpp new file mode 100644 index 00000000..45a560de --- /dev/null +++ b/logic/settings/PassthroughSetting.cpp @@ -0,0 +1,66 @@ +/* Copyright 2013-2015 MultiMC Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "PassthroughSetting.h" + +PassthroughSetting::PassthroughSetting(std::shared_ptr other, std::shared_ptr gate) + : Setting(other->configKeys(), QVariant()) +{ + Q_ASSERT(other); + Q_ASSERT(gate); + m_other = other; + m_gate = gate; +} + +bool PassthroughSetting::isOverriding() const +{ + return m_gate->get().toBool(); +} + +QVariant PassthroughSetting::defValue() const +{ + if(isOverriding()) + { + return m_other->get(); + } + return m_other->defValue(); +} + +QVariant PassthroughSetting::get() const +{ + if(isOverriding()) + { + return Setting::get(); + } + return m_other->get(); +} + +void PassthroughSetting::reset() +{ + if(isOverriding()) + { + Setting::reset(); + } + m_other->reset(); +} + +void PassthroughSetting::set(QVariant value) +{ + if(isOverriding()) + { + Setting::set(value); + } + m_other->set(value); +} diff --git a/logic/settings/PassthroughSetting.h b/logic/settings/PassthroughSetting.h new file mode 100644 index 00000000..c4dc646c --- /dev/null +++ b/logic/settings/PassthroughSetting.h @@ -0,0 +1,45 @@ +/* Copyright 2013-2015 MultiMC Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include + +#include "Setting.h" + +/*! + * \brief A setting that 'overrides another.' based on the value of a 'gate' setting + * If 'gate' evaluates to true, the override stores and returns data + * If 'gate' evaluates to false, the original does, + */ +class PassthroughSetting : public Setting +{ + Q_OBJECT +public: + explicit PassthroughSetting(std::shared_ptr overriden, std::shared_ptr gate); + + virtual QVariant defValue() const; + virtual QVariant get() const; + virtual void set (QVariant value); + virtual void reset(); + +private: + bool isOverriding() const; + +protected: + std::shared_ptr m_other; + std::shared_ptr m_gate; +}; diff --git a/logic/settings/SettingsObject.cpp b/logic/settings/SettingsObject.cpp index e88eae49..5e682cbf 100644 --- a/logic/settings/SettingsObject.cpp +++ b/logic/settings/SettingsObject.cpp @@ -16,6 +16,7 @@ #include "settings/SettingsObject.h" #include "settings/Setting.h" #include "settings/OverrideSetting.h" +#include "PassthroughSetting.h" #include #include @@ -44,6 +45,22 @@ std::shared_ptr SettingsObject::registerOverride(std::shared_ptr SettingsObject::registerPassthrough(std::shared_ptr original, + std::shared_ptr gate) +{ + if (contains(original->id())) + { + qCritical() << QString("Failed to register setting %1. ID already exists.") + .arg(original->id()); + return nullptr; // Fail + } + auto passthrough = std::make_shared(original, gate); + passthrough->m_storage = this; + connectSignals(*passthrough); + m_settings.insert(passthrough->id(), passthrough); + return passthrough; +} + std::shared_ptr SettingsObject::registerSetting(QStringList synonyms, QVariant defVal) { if (synonyms.empty()) diff --git a/logic/settings/SettingsObject.h b/logic/settings/SettingsObject.h index ac8419e8..99782a01 100644 --- a/logic/settings/SettingsObject.h +++ b/logic/settings/SettingsObject.h @@ -50,6 +50,16 @@ public: */ std::shared_ptr registerOverride(std::shared_ptr original); + /*! + * Registers a passthorugh setting for the given original setting in this settings object + * gate decides if the passthrough (true) or the original (false) is used for value + * + * This will fail if there is already a setting with the same ID as + * the one that is being registered. + * \return A valid Setting shared pointer if successful. + */ + std::shared_ptr registerPassthrough(std::shared_ptr original, std::shared_ptr gate); + /*! * Registers the given setting with this SettingsObject and connects the necessary signals. * -- cgit v1.2.3