From 54fd26fee15c320c363a4be355c80e4b7ce38164 Mon Sep 17 00:00:00 2001 From: Eran Mizrahi Date: Fri, 8 Dec 2017 11:00:57 +0200 Subject: new diagnostics settings dialog tab --- src/settings.cpp | 37 +++++++-- src/settings.h | 25 +++++- src/settingsdialog.ui | 225 ++++++++++++++++++++++++++++++++++++++++---------- 3 files changed, 237 insertions(+), 50 deletions(-) (limited to 'src') diff --git a/src/settings.cpp b/src/settings.cpp index fc740302..7d90d861 100644 --- a/src/settings.cpp +++ b/src/settings.cpp @@ -28,6 +28,7 @@ along with Mod Organizer. If not, see . #include #include #include +#include #include #include @@ -38,6 +39,7 @@ along with Mod Organizer. If not, see . #include #include #include +#include #include #include #include @@ -334,9 +336,18 @@ bool Settings::offlineMode() const int Settings::logLevel() const { - return m_Settings.value("Settings/log_level", 0).toInt(); + return m_Settings.value("Settings/log_level", static_cast(LogLevel::Info)).toInt(); } +int Settings::crashDumpsType() const +{ + return m_Settings.value("Settings/crash_dumps_type", static_cast(CrashDumpsType::Mini)).toInt(); +} + +int Settings::crashDumpsMax() const +{ + return m_Settings.value("Settings/crash_dumps_max", 5).toInt(); +} void Settings::setNexusLogin(QString username, QString password) { @@ -581,6 +592,7 @@ void Settings::query(QWidget *parent) tabs.push_back(std::unique_ptr(new GeneralTab(this, dialog))); tabs.push_back(std::unique_ptr(new PathsTab(this, dialog))); + tabs.push_back(std::unique_ptr(new DiagnosticsTab(this, dialog))); tabs.push_back(std::unique_ptr(new NexusTab(this, dialog))); tabs.push_back(std::unique_ptr(new SteamTab(this, dialog))); tabs.push_back(std::unique_ptr(new PluginsTab(this, dialog))); @@ -608,7 +620,6 @@ Settings::GeneralTab::GeneralTab(Settings *m_parent, SettingsDialog &m_dialog) : Settings::SettingsTab(m_parent, m_dialog) , m_languageBox(m_dialog.findChild("languageBox")) , m_styleBox(m_dialog.findChild("styleBox")) - , m_logLevelBox(m_dialog.findChild("logLevelBox")) , m_compactBox(m_dialog.findChild("compactBox")) , m_showMetaBox(m_dialog.findChild("showMetaBox")) , m_usePrereleaseBox(m_dialog.findChild("usePrereleaseBox")) @@ -640,7 +651,6 @@ Settings::GeneralTab::GeneralTab(Settings *m_parent, SettingsDialog &m_dialog) } } - m_logLevelBox->setCurrentIndex(m_parent->logLevel()); m_compactBox->setChecked(m_parent->compactDownloads()); m_showMetaBox->setChecked(m_parent->metaDownloads()); m_usePrereleaseBox->setChecked(m_parent->usePrereleases()); @@ -662,9 +672,6 @@ void Settings::GeneralTab::update() emit m_parent->styleChanged(newStyle); } - m_Settings.setValue("Settings/log_level", m_logLevelBox->currentIndex()); - - m_Settings.setValue("Settings/compact_downloads", m_compactBox->isChecked()); m_Settings.setValue("Settings/meta_downloads", m_showMetaBox->isChecked()); m_Settings.setValue("Settings/use_prereleases", m_usePrereleaseBox->isChecked()); @@ -743,6 +750,24 @@ void Settings::PathsTab::update() } } +Settings::DiagnosticsTab::DiagnosticsTab(Settings *m_parent, SettingsDialog &m_dialog) + : Settings::SettingsTab(m_parent, m_dialog) + , m_logLevelBox(m_dialog.findChild("logLevelBox")) + , m_dumpsTypeBox(m_dialog.findChild("dumpsTypeBox")) + , m_dumpsMaxEdit(m_dialog.findChild("dumpsMaxEdit")) +{ + m_logLevelBox->setCurrentIndex(m_parent->logLevel()); + m_dumpsTypeBox->setCurrentIndex(m_parent->crashDumpsType()); + m_dumpsMaxEdit->setValue(m_parent->crashDumpsMax()); +} + +void Settings::DiagnosticsTab::update() +{ + m_Settings.setValue("Settings/log_level", m_logLevelBox->currentIndex()); + m_Settings.setValue("Settings/crash_dumps_type", m_dumpsTypeBox->currentIndex()); + m_Settings.setValue("Settings/crash_dumps_max", m_dumpsMaxEdit->value()); +} + Settings::NexusTab::NexusTab(Settings *parent, SettingsDialog &dialog) : Settings::SettingsTab(parent, dialog) , m_loginCheckBox(dialog.findChild("loginCheckBox")) diff --git a/src/settings.h b/src/settings.h index d316c82d..9ee29ba3 100644 --- a/src/settings.h +++ b/src/settings.h @@ -38,6 +38,7 @@ along with Mod Organizer. If not, see . class QCheckBox; class QComboBox; class QLineEdit; +class QSpinBox; class QListWidget; class QWidget; @@ -204,6 +205,16 @@ public: */ int logLevel() const; + /** + * @return the configured crash dumps type + */ + int crashDumpsType() const; + + /** + * @return the configured crash dumps max + */ + int crashDumpsMax() const; + /** * @brief set the nexus login information * @@ -371,7 +382,6 @@ private: private: QComboBox *m_languageBox; QComboBox *m_styleBox; - QComboBox *m_logLevelBox; QCheckBox *m_compactBox; QCheckBox *m_showMetaBox; QCheckBox *m_usePrereleaseBox; @@ -393,6 +403,19 @@ private: QLineEdit *m_overwriteDirEdit; }; + class DiagnosticsTab : public SettingsTab + { + public: + DiagnosticsTab(Settings *parent, SettingsDialog &dialog); + + void update(); + + private: + QComboBox *m_logLevelBox; + QComboBox *m_dumpsTypeBox; + QSpinBox *m_dumpsMaxEdit; + }; + /** Display/store the configuration in the 'nexus' tab of the settings dialogue */ class NexusTab : public SettingsTab { diff --git a/src/settingsdialog.ui b/src/settingsdialog.ui index f5e37ea6..632dc6c9 100644 --- a/src/settingsdialog.ui +++ b/src/settingsdialog.ui @@ -70,48 +70,6 @@ p, li { white-space: pre-wrap; } - - - - - - Log Level - - - - - - - Decides the amount of data printed to "ModOrganizer.log" - - - Decides the amount of data printed to "ModOrganizer.log". -"Debug" produces very useful information for finding problems. There is usually no noteworthy performance impact but the file may become rather large. If this is a problem you may prefer the "Info" level for regluar use. On the "Error" level the log file usually remains empty. - - - - Debug - - - - - Info - - - - - Warning - - - - - Error - - - - - - @@ -378,7 +336,188 @@ If you use pre-releases, never contact me directly by e-mail or via private mess - + + + Diagnostics + + + + + + + + Log Level + + + + + + + Decides the amount of data printed to "ModOrganizer.log" + + + + Decides the amount of data printed to "ModOrganizer.log". + "Debug" produces very useful information for finding problems. There is usually no noteworthy performance impact but the file may become rather large. If this is a problem you may prefer the "Info" level for regluar use. On the "Error" level the log file usually remains empty. + + + + + Debug + + + + + Info (recommended) + + + + + Warning + + + + + Error + + + + + + + + + + Qt::Vertical + + + + 20 + 40 + + + + + + + + + + Crash Dumps + + + + + + + Decides which type of crash dumps are collected when injected processes crash. + + + + Decides which type of crash dumps are collected when injected processes crash. + "None" Disables the generation of crash dumps by MO. + "Mini" Default level which generates small dumps (only stack traces). + "Data" Much larger dumps with additional information which may be need (also data segments). + "Full" Even larger dumps with a full memory dump of the process. + + + + + None + + + + + Mini (recommended) + + + + + Data + + + + + Full + + + + + + + + + + + + Max Dumps To Keep + + + + + + + Qt::Horizontal + + + + 60 + 20 + + + + + + + + Maximum number of crash dumps to keep on disk. Use 0 for unlimited. + + + + Maximum number of crash dumps to keep on disk. Use 0 for unlimited. + Set "Crash Dumps" above to None to disable crash dump collection. + + + + + + + + + + + Dumps are stored in <a href="%LOCALAPPDATA%\modorganizer">%LOCALAPPDATA%\modorganizer</a>. + Sending such dumps to the developers can help solve crashes caused by MO. + It is recommended to compress the dumps before sending, especially on the larger settings. + + + + true + + + Hint: right click link and copy link location + + + + + + + Qt::Vertical + + + QSizePolicy::Expanding + + + + 20 + 232 + + + + + + + Nexus -- cgit v1.3.1 From 4adb13c9435baade511d3ac78fb142dc73bd2e1f Mon Sep 17 00:00:00 2001 From: Eran Mizrahi Date: Fri, 8 Dec 2017 18:24:06 +0200 Subject: generate dumps using new diagnostics settings --- src/main.cpp | 79 ++++++++++---------------------------------------- src/mainwindow.cpp | 2 +- src/organizercore.cpp | 23 +++++++++++++-- src/organizercore.h | 7 ++++- src/settings.cpp | 2 +- src/usvfsconnector.cpp | 36 ++++++++++++++++------- src/usvfsconnector.h | 4 ++- 7 files changed, 73 insertions(+), 80 deletions(-) (limited to 'src') diff --git a/src/main.cpp b/src/main.cpp index 526563a2..71fbc943 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -48,6 +48,7 @@ along with Mod Organizer. If not, see . #include #include +#include #include #include @@ -127,71 +128,17 @@ bool isNxmLink(const QString &link) static LONG WINAPI MyUnhandledExceptionFilter(struct _EXCEPTION_POINTERS *exceptionPtrs) { - typedef BOOL (WINAPI *FuncMiniDumpWriteDump)(HANDLE process, DWORD pid, HANDLE file, MINIDUMP_TYPE dumpType, - const PMINIDUMP_EXCEPTION_INFORMATION exceptionParam, - const PMINIDUMP_USER_STREAM_INFORMATION userStreamParam, - const PMINIDUMP_CALLBACK_INFORMATION callbackParam); - LONG result = EXCEPTION_CONTINUE_SEARCH; - - HMODULE dbgDLL = ::LoadLibrary(L"dbghelp.dll"); - - static const int errorLen = 200; - char errorBuffer[errorLen + 1]; - memset(errorBuffer, '\0', errorLen + 1); - - if (dbgDLL) { - FuncMiniDumpWriteDump funcDump = (FuncMiniDumpWriteDump)::GetProcAddress(dbgDLL, "MiniDumpWriteDump"); - if (funcDump) { - QString dataPath = qApp->property("dataPath").toString(); - QString exeName = QFileInfo(qApp->applicationFilePath()).fileName(); - QString dumpName = dataPath + "/" + exeName + ".dmp"; - - if (QMessageBox::question(nullptr, QObject::tr("Woops"), - QObject::tr("ModOrganizer has crashed! " - "Should a diagnostic file be created? " - "If you send me this file (%1) to modorganizer@gmail.com, " - "the bug is a lot more likely to be fixed. " - "Please include a short description of what you were " - "doing when the crash happened" - ).arg(dumpName), - QMessageBox::Yes | QMessageBox::No) == QMessageBox::Yes) { - - HANDLE dumpFile = ::CreateFile(dumpName.toStdWString().c_str(), - GENERIC_WRITE, FILE_SHARE_WRITE, nullptr, - CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr); - if (dumpFile != INVALID_HANDLE_VALUE) { - _MINIDUMP_EXCEPTION_INFORMATION exceptionInfo; - exceptionInfo.ThreadId = ::GetCurrentThreadId(); - exceptionInfo.ExceptionPointers = exceptionPtrs; - exceptionInfo.ClientPointers = false; - - BOOL success = funcDump(::GetCurrentProcess(), ::GetCurrentProcessId(), dumpFile, - MiniDumpNormal, &exceptionInfo, nullptr, nullptr); - - ::FlushFileBuffers(dumpFile); - ::CloseHandle(dumpFile); - if (success) { - return EXCEPTION_EXECUTE_HANDLER; - } - _snprintf(errorBuffer, errorLen, "failed to save minidump to %ls (error %lu)", - dumpName.toStdWString().c_str(), ::GetLastError()); - } else { - _snprintf(errorBuffer, errorLen, "failed to create %ls (error %lu)", - dumpName.toStdWString().c_str(), ::GetLastError()); - } - } else { - return result; - } - } else { - _snprintf(errorBuffer, errorLen, "dbghelp.dll outdated"); - } - } else { - _snprintf(errorBuffer, errorLen, "dbghelp.dll not found"); + const std::wstring& dumpPath = OrganizerCore::crashDumpsPath(); + int dumpRes = + CreateMiniDump(exceptionPtrs, OrganizerCore::getGlobalCrashDumpsType(), dumpPath.c_str()); + if (!dumpRes) { + qCritical("ModOrganizer has crashed, crash dump created."); + return EXCEPTION_EXECUTE_HANDLER; + } + else { + qCritical("ModOrganizer has crashed, CreateMiniDump failed (%d, error %lu).", dumpRes, GetLastError()); + return EXCEPTION_CONTINUE_SEARCH; } - - QMessageBox::critical(nullptr, QObject::tr("Woops"), - QObject::tr("ModOrganizer has crashed! Unfortunately I was not able to write a diagnostic file: %1").arg(errorBuffer)); - return result; } static bool HaveWriteAccess(const std::wstring &path) @@ -448,6 +395,10 @@ int runApplication(MOApplication &application, SingleInstance &instance, QSettings settings(dataPath + "/" + QString::fromStdWString(AppConfig::iniFileName()), QSettings::IniFormat); + + // global crashDumpType sits in OrganizerCore to make a bit less ugly to update it when the settings are changed during runtime + OrganizerCore::setGlobalCrashDumpsType(settings.value("Settings/crash_dumps_type", static_cast(CrashDumpsType::Mini)).toInt()); + qDebug("initializing core"); OrganizerCore organizer(settings); if (!organizer.bootstrap()) { diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 48dfdfbd..cab4c41f 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -3296,7 +3296,7 @@ void MainWindow::on_actionSettings_triggered() updateDownloadListDelegate(); - m_OrganizerCore.setLogLevel(settings.logLevel()); + m_OrganizerCore.updateVFSParams(settings.logLevel(), settings.crashDumpsType()); } diff --git a/src/organizercore.cpp b/src/organizercore.cpp index f8a368c9..adb895ec 100644 --- a/src/organizercore.cpp +++ b/src/organizercore.cpp @@ -47,6 +47,7 @@ #include // for qPrintable, etc #include +#include #include // for _tcsicmp #include @@ -66,6 +67,9 @@ using namespace MOShared; using namespace MOBase; +//static +CrashDumpsType OrganizerCore::m_globalCrashDumpsType = CrashDumpsType::None; + static bool isOnline() { QList interfaces = QNetworkInterface::allInterfaces(); @@ -643,8 +647,23 @@ void OrganizerCore::prepareVFS() m_USVFS.updateMapping(fileMapping(m_CurrentProfile->name(), QString())); } -void OrganizerCore::setLogLevel(int logLevel) { - m_USVFS.setLogLevel(logLevel); +void OrganizerCore::updateVFSParams(int logLevel, int crashDumpsType) { + setGlobalCrashDumpsType(crashDumpsType); + m_USVFS.updateParams(logLevel, crashDumpsType); +} + +//static +void OrganizerCore::setGlobalCrashDumpsType(int crashDumpsType) { + m_globalCrashDumpsType = ::crashDumpsType(crashDumpsType); +} + +//static +std::wstring OrganizerCore::crashDumpsPath() { + wchar_t appDataLocal[MAX_PATH]{ 0 }; + ::SHGetFolderPathW(NULL, CSIDL_LOCAL_APPDATA, NULL, 0, appDataLocal); + std::wstring dumpPath{ appDataLocal }; + dumpPath += L"\\modorganizer"; + return dumpPath; } void OrganizerCore::setCurrentProfile(const QString &profileName) diff --git a/src/organizercore.h b/src/organizercore.h index 297b94f6..7bb7faa7 100644 --- a/src/organizercore.h +++ b/src/organizercore.h @@ -158,7 +158,11 @@ public: void prepareVFS(); - void setLogLevel(int logLevel); + void updateVFSParams(int logLevel, int crashDumpsType); + + static CrashDumpsType getGlobalCrashDumpsType() { return m_globalCrashDumpsType; } + static void setGlobalCrashDumpsType(int crashDumpsType); + static std::wstring crashDumpsPath(); public: MOBase::IModRepositoryBridge *createNexusBridge() const; @@ -319,6 +323,7 @@ private: MOBase::DelayedFileWriter m_PluginListsWriter; UsvfsConnector m_USVFS; + static CrashDumpsType m_globalCrashDumpsType; }; #endif // ORGANIZERCORE_H diff --git a/src/settings.cpp b/src/settings.cpp index 7d90d861..05a62591 100644 --- a/src/settings.cpp +++ b/src/settings.cpp @@ -28,7 +28,7 @@ along with Mod Organizer. If not, see . #include #include #include -#include +#include #include #include diff --git a/src/usvfsconnector.cpp b/src/usvfsconnector.cpp index 0420ddc2..b90784d9 100644 --- a/src/usvfsconnector.cpp +++ b/src/usvfsconnector.cpp @@ -19,6 +19,8 @@ along with Mod Organizer. If not, see . #include "usvfsconnector.h" #include "settings.h" +#include "organizercore.h" +#include "shared/util.h" #include #include #include @@ -27,6 +29,7 @@ along with Mod Organizer. If not, see . #include #include #include +#include static const char SHMID[] = "mod_organizer_instance"; @@ -101,14 +104,33 @@ LogLevel logLevel(int level) } } +CrashDumpsType crashDumpsType(int type) +{ + switch (type) { + case CrashDumpsType::Mini: + return CrashDumpsType::Mini; + case CrashDumpsType::Data: + return CrashDumpsType::Data; + case CrashDumpsType::Full: + return CrashDumpsType::Full; + default: + return CrashDumpsType::None; + } +} + UsvfsConnector::UsvfsConnector() { USVFSParameters params; LogLevel level = logLevel(Settings::instance().logLevel()); - USVFSInitParameters(¶ms, SHMID, false, level); + CrashDumpsType dumpType = crashDumpsType(Settings::instance().crashDumpsType()); + + std::string dumpPath = MOShared::ToString(OrganizerCore::crashDumpsPath(), true); + USVFSInitParameters(¶ms, SHMID, false, level, dumpType, dumpPath.c_str()); InitLogging(false); + + qDebug("Initializing VFS <%s, %d, %d, %s>", params.instanceName, params.logLevel, params.crashDumpsType, params.crashDumpsPath); + CreateVFS(¶ms); - SetLogLevel(level); BlacklistExecutable(L"TSVNCache.exe"); @@ -165,12 +187,6 @@ void UsvfsConnector::updateMapping(const MappingType &mapping) */ } -void UsvfsConnector::setLogLevel(int logLevel) { - switch (logLevel) { - case LogLevel::Debug: SetLogLevel(LogLevel::Debug); break; - case LogLevel::Info: SetLogLevel(LogLevel::Info); break; - case LogLevel::Warning: SetLogLevel(LogLevel::Warning); break; - case LogLevel::Error: SetLogLevel(LogLevel::Error); break; - default: SetLogLevel(LogLevel::Debug); break; - } +void UsvfsConnector::updateParams(int logLevel, int crashDumpsType) { + USVFSUpdateParams(::logLevel(logLevel), ::crashDumpsType(crashDumpsType)); } diff --git a/src/usvfsconnector.h b/src/usvfsconnector.h index 8f723a01..0935bac1 100644 --- a/src/usvfsconnector.h +++ b/src/usvfsconnector.h @@ -27,6 +27,7 @@ along with Mod Organizer. If not, see . #include #include #include +#include class LogWorker : public QThread { @@ -67,7 +68,7 @@ public: ~UsvfsConnector(); void updateMapping(const MappingType &mapping); - void setLogLevel(int logLevel); + void updateParams(int logLevel, int crashDumpsType); private: @@ -76,5 +77,6 @@ private: }; +CrashDumpsType crashDumpsType(int type); #endif // USVFSCONNECTOR_H -- cgit v1.3.1 From 1b4b6dbeb46aa7e6b87b3de32cd8953fa0d32a7a Mon Sep 17 00:00:00 2001 From: Eran Mizrahi Date: Sat, 9 Dec 2017 01:56:16 +0200 Subject: cycle crash dumps --- src/mainwindow.cpp | 1 + src/organizercore.cpp | 10 +++++++++- src/organizercore.h | 2 ++ 3 files changed, 12 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index cab4c41f..b47e9c57 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -3297,6 +3297,7 @@ void MainWindow::on_actionSettings_triggered() updateDownloadListDelegate(); m_OrganizerCore.updateVFSParams(settings.logLevel(), settings.crashDumpsType()); + m_OrganizerCore.cycleDiagnostics(); } diff --git a/src/organizercore.cpp b/src/organizercore.cpp index adb895ec..7f86f2b4 100644 --- a/src/organizercore.cpp +++ b/src/organizercore.cpp @@ -630,7 +630,8 @@ bool OrganizerCore::bootstrap() { return createDirectory(m_Settings.getProfileDirectory()) && createDirectory(m_Settings.getModDirectory()) && createDirectory(m_Settings.getDownloadDirectory()) && - createDirectory(m_Settings.getOverwriteDirectory()); + createDirectory(m_Settings.getOverwriteDirectory()) && + createDirectory(QString::fromStdWString(crashDumpsPath())) && cycleDiagnostics(); } void OrganizerCore::createDefaultProfile() @@ -652,6 +653,12 @@ void OrganizerCore::updateVFSParams(int logLevel, int crashDumpsType) { m_USVFS.updateParams(logLevel, crashDumpsType); } +bool OrganizerCore::cycleDiagnostics() { + if (int maxDumps = settings().crashDumpsMax()) + removeOldFiles(QString::fromStdWString(crashDumpsPath()), "*.dmp", maxDumps, QDir::Time|QDir::Reversed); + return true; +} + //static void OrganizerCore::setGlobalCrashDumpsType(int crashDumpsType) { m_globalCrashDumpsType = ::crashDumpsType(crashDumpsType); @@ -1083,6 +1090,7 @@ void OrganizerCore::spawnBinary(const QFileInfo &binary, const QString &argument refreshESPList(); savePluginList(); + cycleDiagnostics(); //These callbacks should not fiddle with directoy structure and ESPs. m_FinishedRun(binary.absoluteFilePath(), processExitCode); diff --git a/src/organizercore.h b/src/organizercore.h index 7bb7faa7..40531195 100644 --- a/src/organizercore.h +++ b/src/organizercore.h @@ -160,6 +160,8 @@ public: void updateVFSParams(int logLevel, int crashDumpsType); + bool cycleDiagnostics(); + static CrashDumpsType getGlobalCrashDumpsType() { return m_globalCrashDumpsType; } static void setGlobalCrashDumpsType(int crashDumpsType); static std::wstring crashDumpsPath(); -- cgit v1.3.1 From b75e3c12318267be1589cee5e13a9fc89b0097b1 Mon Sep 17 00:00:00 2001 From: Eran Mizrahi Date: Sat, 9 Dec 2017 21:38:47 +0200 Subject: Collect crash dumps also for vectored exceptions --- src/main.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/main.cpp b/src/main.cpp index 71fbc943..40224170 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -128,6 +128,12 @@ bool isNxmLink(const QString &link) static LONG WINAPI MyUnhandledExceptionFilter(struct _EXCEPTION_POINTERS *exceptionPtrs) { + if ((exceptionPtrs->ExceptionRecord->ExceptionCode < 0x80000000) // non-critical + || (exceptionPtrs->ExceptionRecord->ExceptionCode == 0xe06d7363)) { // cpp exception + // don't report non-critical exceptions + return EXCEPTION_CONTINUE_SEARCH; + } + const std::wstring& dumpPath = OrganizerCore::crashDumpsPath(); int dumpRes = CreateMiniDump(exceptionPtrs, OrganizerCore::getGlobalCrashDumpsType(), dumpPath.c_str()); @@ -522,7 +528,7 @@ int runApplication(MOApplication &application, SingleInstance &instance, int main(int argc, char *argv[]) { - SetUnhandledExceptionFilter(MyUnhandledExceptionFilter); + AddVectoredExceptionHandler(0, MyUnhandledExceptionFilter); MOApplication application(argc, argv); QStringList arguments = application.arguments(); -- cgit v1.3.1 From 89b1f1d48dd05b372abca64b311b1107f044a897 Mon Sep 17 00:00:00 2001 From: Eran Mizrahi Date: Sat, 9 Dec 2017 01:26:50 +0200 Subject: add logging of process spawn --- src/organizercore.cpp | 4 ++++ src/usvfsconnector.cpp | 8 ++++++++ 2 files changed, 12 insertions(+) (limited to 'src') diff --git a/src/organizercore.cpp b/src/organizercore.cpp index 7f86f2b4..54cfed54 100644 --- a/src/organizercore.cpp +++ b/src/organizercore.cpp @@ -1181,9 +1181,13 @@ HANDLE OrganizerCore::spawnBinaryDirect(const QFileInfo &binary, = QString("launch \"%1\" \"%2\" %3") .arg(QDir::toNativeSeparators(dataCwd), QDir::toNativeSeparators(dataBinPath), arguments); + + qDebug() << "Spawning proxyed process <" << cmdline << ">"; + return startBinary(QFileInfo(QCoreApplication::applicationFilePath()), cmdline, QCoreApplication::applicationDirPath(), true); } else { + qDebug() << "Spawning direct process <" << binary.absoluteFilePath() << "," << arguments << "," << currentDirectory.absolutePath() << ">"; return startBinary(binary, arguments, currentDirectory, true); } } else { diff --git a/src/usvfsconnector.cpp b/src/usvfsconnector.cpp index b90784d9..ffbdf3aa 100644 --- a/src/usvfsconnector.cpp +++ b/src/usvfsconnector.cpp @@ -158,6 +158,10 @@ void UsvfsConnector::updateMapping(const MappingType &mapping) progress.setMaximum(static_cast(mapping.size())); progress.show(); int value = 0; + int files = 0; + int dirs = 0; + + qDebug("Updating VFS mappings..."); ClearVirtualMappings(); @@ -173,11 +177,15 @@ void UsvfsConnector::updateMapping(const MappingType &mapping) (map.createTarget ? LINKFLAG_CREATETARGET : 0) | LINKFLAG_RECURSIVE ); + ++dirs; } else { VirtualLinkFile(map.source.toStdWString().c_str(), map.destination.toStdWString().c_str(), 0); + ++files; } } + + qDebug("VFS mappings updated ", dirs, files); /* size_t dumpSize = 0; CreateVFSDump(nullptr, &dumpSize); -- cgit v1.3.1 From 66b17eea8ac83ee6f7b729974240d995a1e8ed3a Mon Sep 17 00:00:00 2001 From: Eran Mizrahi Date: Sat, 9 Dec 2017 21:39:22 +0200 Subject: Wait for processes run using command line shortcuts --- src/CMakeLists.txt | 1 + src/ilockedwaitingforprocess.h | 13 ++++++++++++ src/iuserinterface.h | 7 ++----- src/lockeddialog.h | 7 ++++--- src/mainwindow.cpp | 21 +++---------------- src/mainwindow.h | 4 +--- src/organizercore.cpp | 46 +++++++++++++++++++++++++++++++++--------- src/organizercore.h | 2 +- 8 files changed, 62 insertions(+), 39 deletions(-) create mode 100644 src/ilockedwaitingforprocess.h (limited to 'src') diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index d603336c..39aabb91 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -169,6 +169,7 @@ SET(organizer_HDRS viewmarkingscrollbar.h plugincontainer.h organizercore.h + ilockedwaitingforprocess.h iuserinterface.h instancemanager.h usvfsconnector.h diff --git a/src/ilockedwaitingforprocess.h b/src/ilockedwaitingforprocess.h new file mode 100644 index 00000000..6a4267d4 --- /dev/null +++ b/src/ilockedwaitingforprocess.h @@ -0,0 +1,13 @@ +#ifndef ILOCKEDWAITINGFORPROCESS_H +#define ILOCKEDWAITINGFORPROCESS_H + +class QString; + +class ILockedWaitingForProcess +{ +public: + virtual bool unlockClicked() = 0; + virtual void setProcessName(QString const &) = 0; +}; + +#endif // ILOCKEDWAITINGFORPROCESS_H diff --git a/src/iuserinterface.h b/src/iuserinterface.h index 0af1c2ac..255c7ac0 100644 --- a/src/iuserinterface.h +++ b/src/iuserinterface.h @@ -3,6 +3,7 @@ #include "modinfo.h" +#include "ilockedwaitingforprocess.h" #include #include #include @@ -27,12 +28,8 @@ public: virtual void displayModInformation(ModInfo::Ptr modInfo, unsigned int index, int tab) = 0; - virtual void lock() = 0; + virtual ILockedWaitingForProcess* lock() = 0; virtual void unlock() = 0; - virtual bool unlockClicked() = 0; - virtual void setProcessName(QString const &) = 0; - - }; #endif // IUSERINTERFACE_H diff --git a/src/lockeddialog.h b/src/lockeddialog.h index 29ac459b..8803efae 100644 --- a/src/lockeddialog.h +++ b/src/lockeddialog.h @@ -20,6 +20,7 @@ along with Mod Organizer. If not, see . #ifndef LOCKEDDIALOG_H #define LOCKEDDIALOG_H +#include "ilockedwaitingforprocess.h" #include // for QDialog #include // for Q_OBJECT, slots #include // for QString @@ -39,7 +40,7 @@ namespace Ui { * data on which Mod Organizer works. After the UI is unlocked (manually or after the * external application closed) MO will refresh all of its data sources **/ -class LockedDialog : public QDialog +class LockedDialog : public QDialog, public ILockedWaitingForProcess { Q_OBJECT @@ -52,13 +53,13 @@ public: * * @return true if the user clicked the unlock button **/ - bool unlockClicked() const { return m_UnlockClicked; } + bool unlockClicked() override { return m_UnlockClicked; } /** * @brief set the name of the process being run * @param name of process */ - void setProcessName(const QString &name); + void setProcessName(const QString &name) override; protected: diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index b47e9c57..1db895fc 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -1465,17 +1465,18 @@ void MainWindow::storeSettings(QSettings &settings) { } } -void MainWindow::lock() +ILockedWaitingForProcess* MainWindow::lock() { if (m_LockDialog != nullptr) { ++m_LockCount; - return; + return m_LockDialog; } m_LockDialog = new LockedDialog(qApp->activeWindow()); m_LockDialog->show(); setEnabled(false); m_LockDialog->setEnabled(true); //What's the point otherwise? ++m_LockCount; + return m_LockDialog; } void MainWindow::unlock() @@ -1494,22 +1495,6 @@ void MainWindow::unlock() } } -bool MainWindow::unlockClicked() -{ - if (m_LockDialog != nullptr) { - return m_LockDialog->unlockClicked(); - } else { - return false; - } -} - -void MainWindow::setProcessName(QString const &name) -{ - if (m_LockDialog != nullptr) { - m_LockDialog->setProcessName(name); - } -} - void MainWindow::on_btnRefreshData_clicked() { m_OrganizerCore.refreshDirectoryStructure(); diff --git a/src/mainwindow.h b/src/mainwindow.h index cec6c407..f6f11157 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -116,10 +116,8 @@ public: void storeSettings(QSettings &settings) override; void readSettings(); - virtual void lock() override; + virtual ILockedWaitingForProcess* lock() override; virtual void unlock() override; - virtual bool unlockClicked() override; - virtual void setProcessName(QString const &name) override; bool addProfile(); void refreshDataTree(); diff --git a/src/organizercore.cpp b/src/organizercore.cpp index 54cfed54..f0dfa8d4 100644 --- a/src/organizercore.cpp +++ b/src/organizercore.cpp @@ -31,6 +31,7 @@ #include "appconfig.h" #include #include +#include "lockeddialog.h" #include #include @@ -1067,8 +1068,9 @@ QStringList OrganizerCore::modsSortedByProfilePriority() const void OrganizerCore::spawnBinary(const QFileInfo &binary, const QString &arguments, const QDir ¤tDirectory, const QString &steamAppID, const QString &customOverwrite) { + ILockedWaitingForProcess* uilock = nullptr; if (m_UserInterface != nullptr) { - m_UserInterface->lock(); + uilock = m_UserInterface->lock(); } ON_BLOCK_EXIT([&] () { if (m_UserInterface != nullptr) { m_UserInterface->unlock(); } @@ -1077,7 +1079,7 @@ void OrganizerCore::spawnBinary(const QFileInfo &binary, const QString &argument HANDLE processHandle = spawnBinaryDirect(binary, arguments, m_CurrentProfile->name(), currentDirectory, steamAppID, customOverwrite); if (processHandle != INVALID_HANDLE_VALUE) { DWORD processExitCode; - (void)waitForProcessCompletion(processHandle, &processExitCode); + (void)waitForProcessCompletion(processHandle, &processExitCode, uilock); refreshDirectoryStructure(); // need to remove our stored load order because it may be outdated if a foreign tool changed the @@ -1258,24 +1260,50 @@ HANDLE OrganizerCore::startApplication(const QString &executable, } } - return spawnBinaryDirect(binary, arguments, profileName, currentDirectory, - steamAppID, customOverwrite); + HANDLE processHandle = spawnBinaryDirect(binary, arguments, profileName, currentDirectory, steamAppID, customOverwrite); + if (processHandle != INVALID_HANDLE_VALUE) { + std::unique_ptr dlg; + ILockedWaitingForProcess* uilock = nullptr; + + if (m_UserInterface != nullptr) { + uilock = m_UserInterface->lock(); + } + else { + // i.e. when running command line shortcuts there is no m_UserInterface + dlg.reset(new LockedDialog); + dlg->show(); + dlg->setEnabled(true); + uilock = dlg.get(); + } + + ON_BLOCK_EXIT([&]() { + if (m_UserInterface != nullptr) { + m_UserInterface->unlock(); + } }); + + DWORD processExitCode; + waitForProcessCompletion(processHandle, &processExitCode, uilock); + cycleDiagnostics(); + } + + return processHandle; } bool OrganizerCore::waitForApplication(HANDLE handle, LPDWORD exitCode) { + ILockedWaitingForProcess* uilock = nullptr; if (m_UserInterface != nullptr) { - m_UserInterface->lock(); + uilock = m_UserInterface->lock(); } ON_BLOCK_EXIT([&] () { if (m_UserInterface != nullptr) { m_UserInterface->unlock(); } }); - return waitForProcessCompletion(handle, exitCode); + return waitForProcessCompletion(handle, exitCode, uilock); } -bool OrganizerCore::waitForProcessCompletion(HANDLE handle, LPDWORD exitCode) +bool OrganizerCore::waitForProcessCompletion(HANDLE handle, LPDWORD exitCode, ILockedWaitingForProcess* uilock) { DWORD startPID = ::GetProcessId(handle); @@ -1292,7 +1320,7 @@ bool OrganizerCore::waitForProcessCompletion(HANDLE handle, LPDWORD exitCode) while ( res = ::MsgWaitForMultipleObjects(1, &handle, false, 500, QS_KEY | QS_MOUSEBUTTON), - ((m_UserInterface == nullptr) || !m_UserInterface->unlockClicked())) { + ((uilock == nullptr) || !uilock->unlockClicked())) { if (!::GetVFSProcessList(&numProcesses, processes)) { break; @@ -1335,7 +1363,7 @@ bool OrganizerCore::waitForProcessCompletion(HANDLE handle, LPDWORD exitCode) // Update the lock process name with the name of the lowest active PID - though this may not actually be the main process if (handles.size() > 0) - m_UserInterface->setProcessName(QString::fromStdWString(getProcessName(handles.begin()->second))); + uilock->setProcessName(QString::fromStdWString(getProcessName(handles.begin()->second))); // If the main wait process dies, we need a backup wait process until the subprocesses close if ((res == WAIT_FAILED) || (res == WAIT_OBJECT_0)) { diff --git a/src/organizercore.h b/src/organizercore.h index 40531195..0927c88e 100644 --- a/src/organizercore.h +++ b/src/organizercore.h @@ -267,7 +267,7 @@ private: const MOShared::DirectoryEntry *directoryEntry, int createDestination); - bool waitForProcessCompletion(HANDLE handle, LPDWORD exitCode); + bool waitForProcessCompletion(HANDLE handle, LPDWORD exitCode, ILockedWaitingForProcess* uilock); private slots: -- cgit v1.3.1 From 689248619ada12eb9e50ea40963a04c946cb3e13 Mon Sep 17 00:00:00 2001 From: Eran Mizrahi Date: Sat, 9 Dec 2017 23:25:51 +0200 Subject: rewrite waitForProcessCompletion more robustly --- src/organizercore.cpp | 160 +++++++++++++++++++++++--------------------------- 1 file changed, 72 insertions(+), 88 deletions(-) (limited to 'src') diff --git a/src/organizercore.cpp b/src/organizercore.cpp index f0dfa8d4..91330d3a 100644 --- a/src/organizercore.cpp +++ b/src/organizercore.cpp @@ -1305,102 +1305,86 @@ bool OrganizerCore::waitForApplication(HANDLE handle, LPDWORD exitCode) bool OrganizerCore::waitForProcessCompletion(HANDLE handle, LPDWORD exitCode, ILockedWaitingForProcess* uilock) { - DWORD startPID = ::GetProcessId(handle); - - static const DWORD maxCount = 5; - size_t numProcesses = maxCount; - LPDWORD processes = new DWORD[maxCount]; - std::map handles; - - bool tryAgain = true; - DWORD moProcess = -1; - - DWORD res; - // Wait for a an event on the handle, a key press, mouse click or timeout - while ( - res = ::MsgWaitForMultipleObjects(1, &handle, false, 500, - QS_KEY | QS_MOUSEBUTTON), - ((uilock == nullptr) || !uilock->unlockClicked())) { + bool originalHandle = true; + bool newHandle = true; + bool uiunlocked = false; + + constexpr DWORD INPUT_EVENT = WAIT_OBJECT_0 + 1; + DWORD res = WAIT_TIMEOUT; + while (handle != INVALID_HANDLE_VALUE && (newHandle || res == WAIT_TIMEOUT || res == INPUT_EVENT)) + { + if (newHandle) { + QString processName = QString::fromStdWString(getProcessName(handle)); + processName += QString(" (%1)").arg(GetProcessId(handle)); + if (uilock) + uilock->setProcessName(processName); + qDebug() << "Waiting for" + << (originalHandle ? "spawned" : "usvfs") + << "process completion :" << processName.toUtf8().constData(); + newHandle = false; + } - if (!::GetVFSProcessList(&numProcesses, processes)) { - break; - } + // keep processing events so the app doesn't appear dead + QCoreApplication::processEvents(); - // Get USvFS processes, build a handle map, and allow to continue if invalid PIDs are supplied - bool found = false; - size_t count = - std::min(static_cast(maxCount), numProcesses); - for (size_t i = 0; i < count; ++i) { - DWORD currentProcess = processes[i]; - if (currentProcess != moProcess && handles.count(currentProcess) == 0) { - HANDLE currentHandle = ::OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, FALSE, currentProcess); - std::wstring processName = getProcessName(currentHandle); - if (!boost::starts_with(processName, L"ModOrganizer.exe")) { - found = true; - if (currentHandle == nullptr || currentHandle == INVALID_HANDLE_VALUE) continue; - handles.insert(std::pair(currentProcess, currentHandle)); - } - else - { - moProcess = processes[i]; - ::CloseHandle(currentHandle); - } - } - } + // Wait for a an event on the handle, a key press, mouse click or timeout + res = MsgWaitForMultipleObjects(1, &handle, FALSE, 500, QS_KEY | QS_MOUSEBUTTON); + if (res == WAIT_FAILED) { + qWarning() << "Failed waiting for process completion : MsgWaitForMultipleObjects WAIT_FAILED" << GetLastError(); + break; + } - // Clean up tracked handles - for (std::map::iterator checkHandle = handles.begin(); checkHandle != handles.end(); ++checkHandle) { - if (checkHandle->second != nullptr && checkHandle->second != INVALID_HANDLE_VALUE) { - DWORD processExit; - BOOL codeCheck = ::GetExitCodeProcess(checkHandle->second, &processExit); - if (!codeCheck || processExit != STILL_ACTIVE) { - if (!codeCheck) qDebug() << "Checking the process failed: Error Code " << ::GetLastError(); - ::CloseHandle(checkHandle->second); - checkHandle = handles.erase(checkHandle); - } - } - } + if (uilock && uilock->unlockClicked()) { + uiunlocked = true; + break; + } - // Update the lock process name with the name of the lowest active PID - though this may not actually be the main process - if (handles.size() > 0) - uilock->setProcessName(QString::fromStdWString(getProcessName(handles.begin()->second))); - - // If the main wait process dies, we need a backup wait process until the subprocesses close - if ((res == WAIT_FAILED) || (res == WAIT_OBJECT_0)) { - if (handles.size() > 0) { - // By the time we get here, the main wait function should always immediately continue - // Passing in a handle doesn't seem to work for subprocesses - ::MsgWaitForMultipleObjects(0, NULL, FALSE, 500, QS_KEY | QS_MOUSEBUTTON); - } - } + if (res == WAIT_OBJECT_0) { + // process we were waiting on has completed + if (originalHandle && !::GetExitCodeProcess(handle, exitCode)) + qWarning() << "Failed getting exit code of complete process :" << GetLastError(); + CloseHandle(handle); + handle = INVALID_HANDLE_VALUE; + originalHandle = false; + + // if the previous process spawned a child process and immediately exits we may miss it if we check immediately + QThread::msleep(500); + + // search if there is another usvfs process active and if so wait for it + // in theory a querySize of 1 is probably enough since the MO process doesn't seem to be returned by GetVFSProcessList + constexpr size_t querySize = 2; // just to be on the safe side + DWORD pids[querySize]; + size_t found = querySize; + if (!::GetVFSProcessList(&found, pids)) { + qWarning() << "Failed waiting for process completion : GetVFSProcessList failed?!"; + break; + } - // Give the process list a short time to populate - // Required for initial USVFS boot and process switching - if (handles.size() == 0 && !found) { - if (tryAgain) { - tryAgain = false; - QThread::msleep(500); - continue; - } - else { - break; - } - } - else { - tryAgain = true; - } + for (size_t i = 0; i < found; ++i) { + if (pids[i] == GetCurrentProcessId()) + continue; // obviously don't wait for MO process + handle = ::OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION|SYNCHRONIZE, FALSE, pids[i]); + if (handle == INVALID_HANDLE_VALUE) { + qWarning() << "Failed waiting for process completion : OpenProcess failed" << GetLastError(); + continue; + } + newHandle = true; + break; + } + } + } - // keep processing events so the app doesn't appear dead - QCoreApplication::processEvents(); - } + if (res == WAIT_OBJECT_0) + qDebug() << "Waiting for process completion successfull"; + else if (uiunlocked) + qDebug() << "Waiting for process completion aborted by UI"; + else + qDebug() << "Waiting for process completion not successfull :" << res; - //Cleanup - if (handle != INVALID_HANDLE_VALUE) { - ::CloseHandle(handle); - } - delete[] processes; + if (handle != INVALID_HANDLE_VALUE) + ::CloseHandle(handle); - return res == WAIT_OBJECT_0; + return res == WAIT_OBJECT_0; } bool OrganizerCore::onAboutToRun( -- cgit v1.3.1