From 6000f3bb21a42a4d28d068617e0dc642ef739b26 Mon Sep 17 00:00:00 2001 From: xomx Date: Mon, 24 Oct 2022 02:11:23 +0200 Subject: [PATCH] Fix session.xml emptying by forced Windows update restart This fixes both the long standing problem with the emptying of the session.xml file by forced Windows Update restart/shutdown and some potential Notepad++ crashes caused by possible main Notepad++ window blocking at exit. Two main changes to the original design: - WM_QUERYENDSESSION is not used anymore for the tidy-up ops and it always quickly returns TRUE/FALSE to the system as it should. - there is now a safe-guard flag for the session.xml saving at N++ exit, which prevents otherwise possible incorrect overwriting in case of multiple "endsession" messages. Fix #9850, fix #12389, close #12388 --- PowerEditor/src/MISC/Common/FileInterface.cpp | 8 +- PowerEditor/src/Notepad_plus.cpp | 3 + PowerEditor/src/Notepad_plus.h | 7 +- PowerEditor/src/NppBigSwitch.cpp | 308 +++++++++++++----- PowerEditor/src/Parameters.h | 9 +- PowerEditor/src/localization.cpp | 3 + 6 files changed, 236 insertions(+), 102 deletions(-) diff --git a/PowerEditor/src/MISC/Common/FileInterface.cpp b/PowerEditor/src/MISC/Common/FileInterface.cpp index 4915b6eee..2c62bf902 100644 --- a/PowerEditor/src/MISC/Common/FileInterface.cpp +++ b/PowerEditor/src/MISC/Common/FileInterface.cpp @@ -39,7 +39,7 @@ Win32_IO_File::Win32_IO_File(const wchar_t *fname) _hFile = ::CreateFileW(fname, _accessParam, _shareParam, NULL, _dispParam, _attribParam, NULL); NppParameters& nppParam = NppParameters::getInstance(); - if (nppParam.isQueryEndSessionStarted() && nppParam.doNppLogNulContentCorruptionIssue()) + if (nppParam.isEndSessionStarted() && nppParam.doNppLogNulContentCorruptionIssue()) { generic_string issueFn = nppLogNulContentCorruptionIssue; issueFn += TEXT(".log"); @@ -69,7 +69,7 @@ void Win32_IO_File::close() NppParameters& nppParam = NppParameters::getInstance(); - if (nppParam.isQueryEndSessionStarted() && nppParam.doNppLogNulContentCorruptionIssue()) + if (nppParam.isEndSessionStarted() && nppParam.doNppLogNulContentCorruptionIssue()) { generic_string issueFn = nppLogNulContentCorruptionIssue; issueFn += TEXT(".log"); @@ -132,7 +132,7 @@ bool Win32_IO_File::write(const void *wbuf, unsigned long buf_size) NppParameters& nppParam = NppParameters::getInstance(); if (::WriteFile(_hFile, wbuf, buf_size, &bytes_written, NULL) == FALSE) { - if (nppParam.isQueryEndSessionStarted() && nppParam.doNppLogNulContentCorruptionIssue()) + if (nppParam.isEndSessionStarted() && nppParam.doNppLogNulContentCorruptionIssue()) { generic_string issueFn = nppLogNulContentCorruptionIssue; issueFn += TEXT(".log"); @@ -151,7 +151,7 @@ bool Win32_IO_File::write(const void *wbuf, unsigned long buf_size) } else { - if (nppParam.isQueryEndSessionStarted() && nppParam.doNppLogNulContentCorruptionIssue()) + if (nppParam.isEndSessionStarted() && nppParam.doNppLogNulContentCorruptionIssue()) { generic_string issueFn = nppLogNulContentCorruptionIssue; issueFn += TEXT(".log"); diff --git a/PowerEditor/src/Notepad_plus.cpp b/PowerEditor/src/Notepad_plus.cpp index df951e6d6..076dde6f4 100644 --- a/PowerEditor/src/Notepad_plus.cpp +++ b/PowerEditor/src/Notepad_plus.cpp @@ -2057,6 +2057,9 @@ void Notepad_plus::filePrint(bool showDialog) int Notepad_plus::doSaveOrNot(const TCHAR* fn, bool isMulti) { + if ((NppParameters::getInstance()).isEndSessionCritical()) + return IDCANCEL; // simulate Esc-key or Cancel-button as there should not be any big delay / code-flow block + // In case Notepad++ is iconized into notification zone if (!::IsWindowVisible(_pPublicInterface->getHSelf())) { diff --git a/PowerEditor/src/Notepad_plus.h b/PowerEditor/src/Notepad_plus.h index 4d90ee780..83ea030bb 100644 --- a/PowerEditor/src/Notepad_plus.h +++ b/PowerEditor/src/Notepad_plus.h @@ -383,11 +383,8 @@ private: bool _isFileOpening = false; bool _isAdministrator = false; - bool _isEndingSessionButNotReady = false; // If Windows 10 update needs to restart - // and Notepad++ has one (some) dirty document(s) - // and "Enable session snapshot and periodic backup" is not enabled - // then WM_ENDSESSION is send with wParam == FALSE - // in this case this boolean is set true, so Notepad++ will quit and its current session will be saved + bool _isNppSessionSavedAtExit = false; // guard flag, it prevents emptying of the N++ session.xml in case of multiple WM_ENDSESSION or WM_CLOSE messages + ScintillaCtrls _scintillaCtrls4Plugins; std::vector > _hideLinesMarks; diff --git a/PowerEditor/src/NppBigSwitch.cpp b/PowerEditor/src/NppBigSwitch.cpp index fd6667b67..9fea4d9f5 100644 --- a/PowerEditor/src/NppBigSwitch.cpp +++ b/PowerEditor/src/NppBigSwitch.cpp @@ -2059,14 +2059,29 @@ LRESULT Notepad_plus::process(HWND hwnd, UINT message, WPARAM wParam, LPARAM lPa } case WM_QUERYENDSESSION: - case WM_CLOSE: { - if (message == WM_QUERYENDSESSION) - { - nppParam.queryEndSessionStart(); - } + // app should return TRUE or FALSE immediately upon receiving this message, + // and defer any cleanup operations until it receives WM_ENDSESSION (with WPARAM TRUE) - if (nppParam.isQueryEndSessionStarted() && nppParam.doNppLogNulContentCorruptionIssue()) + // for a bigger tidy-up/save operations we can kick off a background thread here to prepare for shutdown + // and when we get the WM_END­SESSION TRUE, we wait there until that background operation completes + // before telling the system, "ok, you can shut down now...", i.e. returning 0 there + + // whatever we do from here - make sure that it is ok for the operation to occur even if the shutdown + // is then subsequently canceled + + // here we could also display a prompt to ask the users whether they want to save their unsaved changes etc., + // but in practice, this is usually not a good idea because if we do not respond to this message + // after a few seconds (e.g. user is away from PC...), the system will shut down without us + + bool isFirstQueryEndSession = !nppParam.isEndSessionStarted(); + bool isForcedShuttingDown = (lParam & ENDSESSION_CRITICAL); + + nppParam.endSessionStart(); + if (isForcedShuttingDown) + nppParam.makeEndSessionCritical(); + + if (nppParam.doNppLogNulContentCorruptionIssue()) { generic_string issueFn = nppLogNulContentCorruptionIssue; issueFn += TEXT(".log"); @@ -2074,7 +2089,7 @@ LRESULT Notepad_plus::process(HWND hwnd, UINT message, WPARAM wParam, LPARAM lPa pathAppend(nppIssueLog, issueFn); string wmqesType = std::to_string(lParam); - if (0 == lParam) + if (lParam == 0) { wmqesType += " - ordinary system shutdown/restart"; } @@ -2088,11 +2103,173 @@ LRESULT Notepad_plus::process(HWND hwnd, UINT message, WPARAM wParam, LPARAM lPa if (lParam & ENDSESSION_LOGOFF) wmqesType += " - ENDSESSION_LOGOFF"; } - string queryEndSession = "WM_QUERYENDSESSION (lParam: " + wmqesType + ") ====================================="; - if (WM_QUERYENDSESSION == message) - writeLog(nppIssueLog.c_str(), queryEndSession.c_str()); + string msg = "WM_QUERYENDSESSION (lParam: " + wmqesType + ") ====================================="; + writeLog(nppIssueLog.c_str(), msg.c_str()); + } + + if (::IsWindowEnabled(hwnd)) + { + if (MainFileManager.getNbDirtyBuffers() > 0) + { + // we have unsaved filebuffer(s), give the user a chance to respond (non-critical shutdown only) + if (!isForcedShuttingDown && isFirstQueryEndSession) + { + // if N++ has been minimized or invisible, we need to show it 1st + if (::IsIconic(hwnd)) + { + ::ShowWindow(hwnd, SW_RESTORE); + } + else + { + if (!::IsWindowVisible(hwnd)) + { + // systray etc... + ::ShowWindow(hwnd, SW_SHOW); + ::SendMessage(hwnd, WM_SIZE, 0, 0); // to make window fit (specially to show tool bar.) + } + } + ::PostMessage(hwnd, WM_COMMAND, IDM_FILE_SAVEALL, 0); // posting will not block us here + return FALSE; // request abort of the shutdown + } + } + } + else + { + // we probably have a blocking modal-window like MessageBox (e.g. the "Reload" or "Keep non existing file") + if (!isForcedShuttingDown && isFirstQueryEndSession) + return FALSE; // request abort of the shutdown (for a non-critical one we can give the user a chance to solve whatever is needed) + + // here is the right place to unblock the modal-dlg blocking the main N++ wnd, because then it will be too late + // to do so at the WM_ENDSESSION time (for that we need this thread message queue...) + + // in most cases we will need to take care and programmatically close such dialogs in order to exit gracefully, + // otherwise the N++ most probably crashes itself without any tidy-up + + string strLog = "Main N++ wnd is disabled by (an active modal-dlg?): "; + char szBuf[MAX_PATH + 128] = { 0 }; + + HWND hActiveWnd = ::GetActiveWindow(); + if (hActiveWnd) + { + if (::GetWindowTextA(hActiveWnd, szBuf, _countof(szBuf))) + strLog += szBuf; + ::SendMessage(hActiveWnd, WM_CLOSE, 0, 0); + } else - writeLog(nppIssueLog.c_str(), "WM_CLOSE (isQueryEndSessionStarted == true)"); + { + // no active child window, so it is most probably the system dialog box class #32770 (used e.g. by the MessageBox WINAPI) + // - so our main hwnd here is not the PARENT but OWNER of that top-level window + hActiveWnd = ::GetLastActivePopup(hwnd); + if (hActiveWnd) + { + if (::GetWindowTextA(hActiveWnd, szBuf, _countof(szBuf))) + strLog += szBuf; + ::GetClassNameA(hActiveWnd, szBuf, _countof(szBuf)); + if (lstrcmpiA("#32770", szBuf) == 0) + strLog += " (MessageBox)"; + // we cannot use here the usual sending of the WM_CLOSE as it will not always work (e.g. for a MB_YESNO MessageBox) + if (!::EndDialog(hActiveWnd, 0)) + { + strLog += " -> EndDialog failed with ErrorCode: "; + strLog += std::to_string(::GetLastError()); + // last attempt + ::SendMessage(hActiveWnd, WM_SYSCOMMAND, SC_CLOSE, 0); + } + } + else + { + strLog += "???"; + } + } + + // re-test + if (::IsWindowEnabled(hwnd)) + strLog += " -> Main N++ wnd has been successfully reenabled."; + + if (nppParam.doNppLogNulContentCorruptionIssue()) + { + generic_string issueFn = nppLogNulContentCorruptionIssue; + issueFn += TEXT(".log"); + generic_string nppIssueLog = nppParam.getUserPath(); + pathAppend(nppIssueLog, issueFn); + writeLog(nppIssueLog.c_str(), strLog.c_str()); + } + } + + // TODO: here is the last opportunity to call the following WINAPI in a possible future version of the N++ + // + // flags RESTART_NO_PATCH and RESTART_NO_REBOOT are not set, so we should be restarted if terminated by an update or restart + //::RegisterApplicationRestart(restartCommandLine.c_str(), RESTART_NO_CRASH | RESTART_NO_HANG); + + return TRUE; // nowadays, with the monstrous Win10+ Windows Update behind, is futile to try to interrupt the shutdown by returning FALSE here + // (if one really needs so, there is the ShutdownBlockReasonCreate WINAPI for the rescue ...) + } + + case WM_ENDSESSION: + { + // this message informs our app whether the session is really ending + + if (nppParam.doNppLogNulContentCorruptionIssue()) + { + generic_string issueFn = nppLogNulContentCorruptionIssue; + issueFn += TEXT(".log"); + generic_string nppIssueLog = nppParam.getUserPath(); + pathAppend(nppIssueLog, issueFn); + + string wmesType = std::to_string(lParam); + if (lParam == 0) + { + wmesType += " - ordinary system shutdown/restart"; + } + else + { + // the lParam here is a bit mask, it can be one or more of the following values + if (lParam & ENDSESSION_CLOSEAPP) + wmesType += " - ENDSESSION_CLOSEAPP"; + if (lParam & ENDSESSION_CRITICAL) + wmesType += " - ENDSESSION_CRITICAL"; + if (lParam & ENDSESSION_LOGOFF) + wmesType += " - ENDSESSION_LOGOFF"; + } + string msg = "WM_ENDSESSION (wParam: "; + if (wParam) + msg += "TRUE, lParam: "; + else + msg += "FALSE, lParam: "; + msg += wmesType + ")"; + + writeLog(nppIssueLog.c_str(), msg.c_str()); + } + + if (wParam == FALSE) + { + // the session is not being ended after all + // - it happens when either the N++ returns FALSE to non-critical WM_QUERYENDSESSION or any other app with higher shutdown level + // than N++ (app shuttdown order can be checked by the GetProcessShutdownParameters WINAPI) + // - we will not try to reset back our nppParam _isEndSessionStarted flag somehow, because of we should now that there was already + // a previous shutdown attempt, otherwise we could stubbornly repeat returning FALSE for the next WM_QUERYENDSESSION and + // the system will terminate us + return 0; // return here and do not continue to the WM_CLOSE part + } + else + { + // the session is being ended, it can end ANY TIME after all apps running have returned from processing this message, + // so DO NOT e.g. Send/Post any message from here onwards!!! + nppParam.endSessionStart(); // ensure + nppParam.makeEndSessionCritical(); // set our exit-flag to critical even if the bitmask has not the ENDSESSION_CRITICAL set + // do not return 0 here and continue to the N++ standard WM_CLOSE code-part (no verbose GUI there this time!!!) + } + } // case WM_ENDSESSION: + + case WM_CLOSE: + { + if (nppParam.doNppLogNulContentCorruptionIssue() && nppParam.isEndSessionStarted() && (message == WM_CLOSE)) + { + generic_string issueFn = nppLogNulContentCorruptionIssue; + issueFn += TEXT(".log"); + generic_string nppIssueLog = nppParam.getUserPath(); + pathAppend(nppIssueLog, issueFn); + writeLog(nppIssueLog.c_str(), "WM_CLOSE (isEndSessionStarted == true)"); } if (_pPublicInterface->isPrelaunch()) @@ -2102,9 +2279,9 @@ LRESULT Notepad_plus::process(HWND hwnd, UINT message, WPARAM wParam, LPARAM lPa else { SCNotification scnN{}; - scnN.nmhdr.code = NPPN_BEFORESHUTDOWN; scnN.nmhdr.hwndFrom = hwnd; scnN.nmhdr.idFrom = 0; + scnN.nmhdr.code = NPPN_BEFORESHUTDOWN; _pluginsManager.notify(&scnN); if (_pTrayIco) @@ -2141,15 +2318,14 @@ LRESULT Notepad_plus::process(HWND hwnd, UINT message, WPARAM wParam, LPARAM lPa saveFileBrowserParam(); saveColumnEditorParams(); - if (!allClosed) + if (!allClosed && !nppParam.isEndSessionCritical()) { - //User cancelled the shutdown + // cancelled by user scnN.nmhdr.code = NPPN_CANCELSHUTDOWN; _pluginsManager.notify(&scnN); - if (isSnapshotMode) ::LockWindowUpdate(NULL); - return FALSE; + return 0; // abort quitting } if (_beforeSpecialView._isFullScreen) //closing, return to windowed mode @@ -2163,7 +2339,6 @@ LRESULT Notepad_plus::process(HWND hwnd, UINT message, WPARAM wParam, LPARAM lPa scnN.nmhdr.code = NPPN_SHUTDOWN; _pluginsManager.notify(&scnN); - saveScintillasZoom(); saveGUIParams(); //writeGUIParams writeScintillaParams saveFindHistory(); //writeFindHistory @@ -2183,18 +2358,23 @@ LRESULT Notepad_plus::process(HWND hwnd, UINT message, WPARAM wParam, LPARAM lPa // saveShortcuts(); - // - // saving session.xml - // - if (nppgui._rememberLastSession && !nppgui._isCmdlineNosessionActivated) - saveSession(currentSession); + if (!_isNppSessionSavedAtExit) + { + _isNppSessionSavedAtExit = true; // prevents emptying of the session.xml file on another WM_ENDSESSION or WM_CLOSE - // - // saving session.xml into loaded session if a saved session is loaded and saveLoadedSessionOnExit option is enabled - // - generic_string loadedSessionFilePath = nppParam.getLoadedSessionFilePath(); - if (!loadedSessionFilePath.empty() && PathFileExists(loadedSessionFilePath.c_str())) - nppParam.writeSession(currentSession, loadedSessionFilePath.c_str()); + // + // saving session.xml + // + if (nppgui._rememberLastSession && !nppgui._isCmdlineNosessionActivated) + saveSession(currentSession); + + // + // saving session.xml into loaded session if a saved session is loaded and saveLoadedSessionOnExit option is enabled + // + generic_string loadedSessionFilePath = nppParam.getLoadedSessionFilePath(); + if (!loadedSessionFilePath.empty() && PathFileExists(loadedSessionFilePath.c_str())) + nppParam.writeSession(currentSession, loadedSessionFilePath.c_str()); + } // write settings on cloud if enabled, if the settings files don't exist if (nppgui._cloudPath != TEXT("") && nppParam.isCloudPathChanged()) @@ -2215,83 +2395,31 @@ LRESULT Notepad_plus::process(HWND hwnd, UINT message, WPARAM wParam, LPARAM lPa ::LockWindowUpdate(NULL); //Sends WM_DESTROY, Notepad++ will end - if (message == WM_CLOSE) - ::DestroyWindow(hwnd); - - generic_string updaterFullPath = nppParam.getWingupFullPath(); - if (!updaterFullPath.empty()) - { - Process updater(updaterFullPath.c_str(), nppParam.getWingupParams().c_str(), nppParam.getWingupDir().c_str()); - updater.run(nppParam.shouldDoUAC()); - } - } - - // _isEndingSessionButNotReady is true means WM_QUERYENDSESSION is sent but no time to finish saving data - // then WM_ENDSESSION is sent with wParam == FALSE - Notepad++ should exit in this case - if (_isEndingSessionButNotReady) ::DestroyWindow(hwnd); - if (message == WM_CLOSE) - return 0; - - return TRUE; - } - - case WM_ENDSESSION: - { - if (nppParam.isQueryEndSessionStarted() && nppParam.doNppLogNulContentCorruptionIssue()) - { - generic_string issueFn = nppLogNulContentCorruptionIssue; - issueFn += TEXT(".log"); - generic_string nppIssueLog = nppParam.getUserPath(); - pathAppend(nppIssueLog, issueFn); - - string wmesType = std::to_string(lParam); - if (0 == lParam) + if (!nppParam.isEndSessionCritical()) { - wmesType += " - ordinary system shutdown/restart"; + generic_string updaterFullPath = nppParam.getWingupFullPath(); + if (!updaterFullPath.empty()) + { + Process updater(updaterFullPath.c_str(), nppParam.getWingupParams().c_str(), nppParam.getWingupDir().c_str()); + updater.run(nppParam.shouldDoUAC()); + } } - else - { - // the lParam here is a bit mask, it can be one or more of the following values - if (lParam & ENDSESSION_CLOSEAPP) - wmesType += " - ENDSESSION_CLOSEAPP"; - if (lParam & ENDSESSION_CRITICAL) - wmesType += " - ENDSESSION_CRITICAL"; - if (lParam & ENDSESSION_LOGOFF) - wmesType += " - ENDSESSION_LOGOFF"; - } - string endSession = "WM_ENDSESSION (wParam: "; - if (wParam) - endSession += "TRUE, lParam: "; - else - endSession += "FALSE, lParam: "; - endSession += wmesType + ")"; - - writeLog(nppIssueLog.c_str(), endSession.c_str()); } - if (wParam == TRUE) - { - ::DestroyWindow(hwnd); - } - else - { - _isEndingSessionButNotReady = true; - } - return 0; + return 0; // both WM_CLOSE and a possible WM_ENDSESSION should return 0 } case WM_DESTROY: { - if (nppParam.isQueryEndSessionStarted() && nppParam.doNppLogNulContentCorruptionIssue()) + if (nppParam.isEndSessionStarted() && nppParam.doNppLogNulContentCorruptionIssue()) { generic_string issueFn = nppLogNulContentCorruptionIssue; issueFn += TEXT(".log"); generic_string nppIssueLog = nppParam.getUserPath(); pathAppend(nppIssueLog, issueFn); - - writeLog(nppIssueLog.c_str(), "WM_DESTROY"); + writeLog(nppIssueLog.c_str(), "WM_DESTROY (isEndSessionStarted == true)"); } killAllChildren(); diff --git a/PowerEditor/src/Parameters.h b/PowerEditor/src/Parameters.h index 60e8f0c0a..2478154ed 100644 --- a/PowerEditor/src/Parameters.h +++ b/PowerEditor/src/Parameters.h @@ -1874,7 +1874,8 @@ private: bool _doNppLogNetworkDriveIssue = false; bool _doNppLogNulContentCorruptionIssue = false; - bool _isQueryEndSessionStarted = false; + bool _isEndSessionStarted = false; + bool _isEndSessionCritical = false; public: generic_string getWingupFullPath() const { return _wingupFullPath; }; @@ -1888,8 +1889,10 @@ public: bool doNppLogNetworkDriveIssue() { return _doNppLogNetworkDriveIssue; }; bool doNppLogNulContentCorruptionIssue() { return _doNppLogNulContentCorruptionIssue; }; - void queryEndSessionStart() { _isQueryEndSessionStarted = true; }; - bool isQueryEndSessionStarted() { return _isQueryEndSessionStarted; }; + void endSessionStart() { _isEndSessionStarted = true; }; + bool isEndSessionStarted() { return _isEndSessionStarted; }; + void makeEndSessionCritical() { _isEndSessionCritical = true; }; + bool isEndSessionCritical() { return _isEndSessionCritical; }; private: void getLangKeywordsFromXmlTree(); diff --git a/PowerEditor/src/localization.cpp b/PowerEditor/src/localization.cpp index 388ed06ff..465891355 100644 --- a/PowerEditor/src/localization.cpp +++ b/PowerEditor/src/localization.cpp @@ -1365,6 +1365,9 @@ generic_string NativeLangSpeaker::getAttrNameStr(const TCHAR *defaultStr, const int NativeLangSpeaker::messageBox(const char *msgBoxTagName, HWND hWnd, const TCHAR *defaultMessage, const TCHAR *defaultTitle, int msgBoxType, int intInfo, const TCHAR *strInfo) { + if ((NppParameters::getInstance()).isEndSessionCritical()) + return IDCANCEL; // simulate Esc-key or Cancel-button as there should not be any big delay / code-flow block + generic_string msg, title; if (!getMsgBoxLang(msgBoxTagName, title, msg)) {