View Issue Details

IDProjectCategoryView StatusLast Update
0001760OpenMPTGeneralpublic2024-04-10 15:50
ReporterSaga Musix Assigned Tomanx  
PrioritylowSeverityminorReproducibilitysometimes
Status resolvedResolutionfixed 
Product VersionOpenMPT 1.31.06.00 / libopenmpt 0.7.6 (current stable) 
Fixed in VersionOpenMPT 1.31.07.00 / libopenmpt 0.7.7 (upcoming stable) 
Summary0001760: Closing OpenMPT while updates are fetched in the background can lead to a crash
Description

When updates are automatically searche for on OpenMPT startup, and the OpenMPT window is closed while the update is still running in the background, we try to access the CMainFrame singleton to send the update result to. However, the main window may already have been destroyed at this point in time, causing a crash. Low severity as the app was to be closed anyway, but it shows that the mechanism should probably be improved.

TagsNo tags attached.
Has the bug occurred in previous versions?
Tested code revision (in case you know it)

Relationships

related to 0001763 assignedmanx rework Update Check thread handling 

Activities

manx

manx

2024-03-24 14:49

administrator   ~0005883

A simple and good solution is surprisingly difficult because we need the message loop to be running for any communication with the update check thread to be possible at all.

update-check-cancel-on-exit-v1.patch (2,720 bytes)   
Index: mptrack/MainFrm.cpp
===================================================================
--- mptrack/MainFrm.cpp	(revision 20463)
+++ mptrack/MainFrm.cpp	(working copy)
@@ -375,6 +375,10 @@
 		}
 	}
 
+#if defined(MPT_ENABLE_UPDATE)
+	m_cancelUpdateCheck = true;
+#endif // MPT_ENABLE_UPDATE
+
 	CChildFrame *pMDIActive = (CChildFrame *)MDIGetActive();
 
 	BeginWaitCursor();
@@ -400,6 +404,10 @@
 		m_InputHandler->m_activeCommandSet->SaveFile(TrackerSettings::Instance().m_szKbdFile);
 	}
 
+#if defined(MPT_ENABLE_UPDATE)
+	CUpdateCheck::WaitForUpdateCheckFinished();
+#endif // MPT_ENABLE_UPDATE
+
 	EndWaitCursor();
 	CMDIFrameWnd::OnClose();
 }
@@ -2765,6 +2773,10 @@
 
 LRESULT CMainFrame::OnUpdateCheckProgress(WPARAM wparam, LPARAM lparam)
 {
+	if(m_cancelUpdateCheck)
+	{
+		return FALSE;
+	}
 	bool isAutoUpdate = wparam != 0;
 	CString updateText = MPT_CFORMAT("Checking for updates... {}%")(lparam);
 	if(isAutoUpdate)
Index: mptrack/Mainfrm.h
===================================================================
--- mptrack/Mainfrm.h	(revision 20463)
+++ mptrack/Mainfrm.h	(working copy)
@@ -191,6 +191,7 @@
 #if defined(MPT_ENABLE_UPDATE)
 	class CUpdateSetupDlg *m_UpdateOptionsDialog = nullptr;
 	std::unique_ptr<UpdateCheckResult> m_updateCheckResult;
+	bool m_cancelUpdateCheck = false;
 #endif // MPT_ENABLE_UPDATE
 	DWORD helpCookie = 0;
 	bool m_bOptionsLocked = false;
Index: mptrack/UpdateCheck.cpp
===================================================================
--- mptrack/UpdateCheck.cpp	(revision 20463)
+++ mptrack/UpdateCheck.cpp	(working copy)
@@ -462,6 +462,24 @@
 }
 
 
+void CUpdateCheck::WaitForUpdateCheckFinished()
+{
+	while(GetNumCurrentRunningInstances() > 0)
+	{
+		MSG msg;
+		while(::PeekMessage(&msg, NULL, 0, 0, PM_REMOVE))
+		{
+			::TranslateMessage(&msg);
+			::DispatchMessage(&msg);
+		}
+		if(GetNumCurrentRunningInstances() > 0)
+		{
+			Sleep(1);
+		}
+	}
+}
+
+
 // Start update check
 void CUpdateCheck::StartUpdateCheckAsync(bool isAutoUpdate)
 {
Index: mptrack/UpdateCheck.h
===================================================================
--- mptrack/UpdateCheck.h	(revision 20463)
+++ mptrack/UpdateCheck.h	(working copy)
@@ -62,7 +62,7 @@
 	static std::vector<mpt::ustring> GetDefaultUpdateSigningKeysRootAnchors();
 	static mpt::ustring GetDefaultAPIURL();
 	
-	int32 GetNumCurrentRunningInstances();
+	static int32 GetNumCurrentRunningInstances();
 
 	static bool IsSuitableUpdateMoment();
 
@@ -69,6 +69,8 @@
 	static void DoAutoUpdateCheck() { StartUpdateCheckAsync(true); }
 	static void DoManualUpdateCheck() { StartUpdateCheckAsync(false); }
 
+	static void WaitForUpdateCheckFinished();
+
 public:
 
 	struct Context
manx

manx

2024-03-26 13:20

administrator   ~0005888

r20470/r20471 fixes the crash.

Issue History

Date Modified Username Field Change
2024-03-24 13:12 Saga Musix New Issue
2024-03-24 13:12 Saga Musix Priority normal => low
2024-03-24 14:49 manx Note Added: 0005883
2024-03-24 14:49 manx File Added: update-check-cancel-on-exit-v1.patch
2024-03-26 13:20 manx Assigned To => manx
2024-03-26 13:20 manx Status new => resolved
2024-03-26 13:20 manx Resolution open => fixed
2024-03-26 13:20 manx Fixed in Version => OpenMPT 1.31.07.00 / libopenmpt 0.7.7 (upcoming stable)
2024-03-26 13:20 manx Note Added: 0005888
2024-03-26 13:21 manx Relationship added related to 0001763