Consolidate cleanup code called on exit

source/hotkey.cpp

@@ -453,71 +453,15 @@ void Hotkey::MaybeUninstallHook()
 
 
 
-void Hotkey::AllDestructAndExit(int aExitCode)
+void Hotkey::AllDestruct()
 {
-	// PostQuitMessage() might be needed to prevent hang-on-exit.  Once this is done, no message boxes or
-	// other dialogs can be displayed.  MSDN: "The exit value returned to the system must be the wParam
-	// parameter of the WM_QUIT message."  In our case, PostQuitMessage() should announce the same exit code
-	// that we will eventually call exit() with:
-	PostQuitMessage(aExitCode);
-
+	// MSDN: "Before terminating, an application must call the UnhookWindowsHookEx function to free
+	// system resources associated with the hook."
 	AddRemoveHooks(0); // Remove all hooks. By contrast, registered hotkeys are unregistered below.
 	if (g_PlaybackHook) // Would be unusual for this to be installed during exit, but should be checked for completeness.
 		UnhookWindowsHookEx(g_PlaybackHook);
 	for (int i = 0; i < sHotkeyCount; ++i)
 		delete shk[i]; // Unregisters before destroying.
-
-	// Do this only at the last possible moment prior to exit() because otherwise
-	// it may free memory that is still in use by objects that depend on it.
-	// This is actually kinda wrong because when exit() is called, the destructors
-	// of static, global, and main-scope objects will be called.  If any of these
-	// destructors try to reference memory freed() by DeleteAll(), there could
-	// be trouble.
-	// It's here mostly for traditional reasons.  I'm 99.99999 percent sure that there would be no
-	// penalty whatsoever to omitting this, since any modern OS will reclaim all
-	// memory dynamically allocated upon program termination.  Indeed, omitting
-	// deletes and free()'s for simple objects will often improve the reliability
-	// and performance since the OS is far more efficient at reclaiming the memory
-	// than us doing it manually (which involves a potentially large number of deletes
-	// due to all the objects and sub-objects to be destructed in a typical C++ program).
-	// UPDATE: In light of the first paragraph above, it seems best not to do this at all,
-	// instead letting all implicitly-called destructors run prior to program termination,
-	// at which time the OS will reclaim all remaining memory:
-	//SimpleHeap::DeleteAll();
-
-	// In light of the comments below, and due to the fact that anyone using this app
-	// is likely to want the anti-focus-stealing measure to always be disabled, I
-	// think it's best not to bother with this ever, since its results are
-	// unpredictable:
-/*	if (g_os.IsWin98orLater() || g_os.IsWin2000orLater())
-		// Restore the original timeout value that was set by WinMain().
-		// Also disables the compiler warning for the PVOID cast.
-		// Note: In many cases, this call will fail to set the value (perhaps even if
-		// SystemParametersInfo() reports success), probably because apps aren't
-		// supposed to change this value unless they currently have the input
-		// focus or something similar (and this app probably doesn't meet the criteria
-		// at this stage).  So I think what will happen is: the value set
-		// in WinMain() will stay in effect until the user reboots, at which time
-		// the default value store in the registry will once again be in effect.
-		// This limitation seems harmless.  Indeed, it's probably a good thing not to
-		// set it back afterward so that windows behave more consistently for the user
-		// regardless of whether this program is currently running.
-#ifdef _MSC_VER
-	#pragma warning( disable : 4312 )
-#endif
-		SystemParametersInfo(SPI_SETFOREGROUNDLOCKTIMEOUT, 0, (PVOID)g_OriginalTimeout, SPIF_SENDCHANGE);
-#ifdef _MSC_VER
-	#pragma warning( default : 4312 ) 
-#endif
-*/
-	// I know this isn't the preferred way to exit the program.  However, due to unusual
-	// conditions such as the script having MsgBoxes or other dialogs displayed on the screen
-	// at the time the user exits (in which case our main event loop would be "buried" underneath
-	// the event loops of the dialogs themselves), this is the only reliable way I've found to exit
-	// so far.  The caller has already called PostQuitMessage(), which might not help but it doesn't hurt:
-	exit(aExitCode); // exit() is insignificant in code size.  It does more than ExitProcess(), but perhaps nothing more that this application actually requires.
-	// By contrast to _exit(), exit() flushes all file buffers before terminating the process. It also
-	// calls any functions registered via atexit or _onexit.
 }
 
 

source/hotkey.h

@@ -202,7 +202,7 @@ class Hotkey
 	static bool sJoystickHasHotkeys[MAX_JOYSTICKS];
 	static DWORD sJoyHotkeyCount;
 
-	static void AllDestructAndExit(int exit_code);
+	static void AllDestruct();
 
 	#define HOTKEY_EL_BADLABEL           _T("1") // Set as strings so that SetFormat doesn't affect their appearance (for use with "If ErrorLevel in 5,6").
 	#define HOTKEY_EL_INVALID_KEYNAME    _T("2")

source/script.cpp

@@ -315,11 +315,20 @@ Script::Script()
 
 Script::~Script() // Destructor.
 {
-	// MSDN: "Before terminating, an application must call the UnhookWindowsHookEx function to free
-	// system resources associated with the hook."
-	AddRemoveHooks(0); // Remove all hooks.
+	Hotkey::AllDestruct(); // Unregister hooks and hotkeys.
+
 	if (mNIC.hWnd) // Tray icon is installed.
 		Shell_NotifyIcon(NIM_DELETE, &mNIC); // Remove it.
+
+	if (mOnClipboardChangeLabel || mOnClipboardChange.Count()) // Remove from viewer chain.
+		EnableClipboardListener(false);
+
+	// DestroyWindow() will cause MainWindowProc() to immediately receive and process the
+	// WM_DESTROY msg, which should in turn result in any child windows being destroyed
+	// and other cleanup being done:
+	g_DestroyWindowCalled = true;
+	DestroyWindow(g_hWnd);
+
 	// Destroy any Progress/SplashImage windows that haven't already been destroyed.  This is necessary
 	// because sometimes these windows aren't owned by the main window:
 	int i;
@@ -395,9 +404,6 @@ Script::~Script() // Destructor.
 	if (g_hFontSplash) // The splash window itself should auto-destroyed, since it's owned by main.
 		DeleteObject(g_hFontSplash);
 
-	if (mOnClipboardChangeLabel || mOnClipboardChange.Count()) // Remove from viewer chain.
-		EnableClipboardListener(false);
-
 	// Close any open sound item to prevent hang-on-exit in certain operating systems or conditions.
 	// If there's any chance that a sound was played and not closed out, or that it is still playing,
 	// this check is done.  Otherwise, the check is avoided since it might be a high overhead call,
@@ -1194,16 +1200,18 @@ void Script::TerminateApp(ExitReasons aExitReason, int aExitCode)
 	g_Debugger.Exit(aExitReason);
 #endif
 
-	// We call DestroyWindow() because MainWindowProc() has left that up to us.
-	// DestroyWindow() will cause MainWindowProc() to immediately receive and process the
-	// WM_DESTROY msg, which should in turn result in any child windows being destroyed
-	// and other cleanup being done:
-	if (IsWindow(g_hWnd)) // Adds peace of mind in case WM_DESTROY was already received in some unusual way.
-	{
-		g_DestroyWindowCalled = true;
-		DestroyWindow(g_hWnd);
-	}
-	Hotkey::AllDestructAndExit(aExitCode);
+	// PostQuitMessage() might be needed to prevent hang-on-exit.  Once this is done, no message boxes or
+	// other dialogs can be displayed.  MSDN: "The exit value returned to the system must be the wParam
+	// parameter of the WM_QUIT message."  In our case, PostQuitMessage() should announce the same exit code
+	// that we will eventually call exit() with:
+	PostQuitMessage(aExitCode);
+
+	// I know this isn't the preferred way to exit the program.  However, due to unusual
+	// conditions such as the script having MsgBoxes or other dialogs displayed on the screen
+	// at the time the user exits (in which case our main event loop would be "buried" underneath
+	// the event loops of the dialogs themselves), this is the only reliable way I've found to exit
+	// so far.
+	exit(aExitCode); // exit() is insignificant in code size.  It does more than ExitProcess(), but perhaps nothing more that this application actually requires.
 }