IridiumIO / CompactGUI

Transparently compress active games and programs using Windows 10/11 APIs
GNU General Public License v3.0
4.71k stars 223 forks source link

Fix a few synchronization-related crashes #415

Closed Eta0 closed 1 week ago

Eta0 commented 3 months ago

Instance Synchronization

This change fixes a few bugs related to synchronization between multiple application instances.

This is my first time working with .NET and with the Visual Basic language, so my bad if any parts of the implementations here are off!

Fix for #413

AbandonedMutexExceptions are now (accurately) treated as successful mutex acquisitions, to prevent a crash loop. Any additional cleanup or scrutiny as to why a previous process exited abruptly is not implemented here.

Fix for #414

The NamedPipeServerStream managed by ProcessNextInstanceMessage is now allowed to shut down gracefully before releasing the application mutex and restarting the process as an administrator, preventing a crash from opening too many server instances for the (unique) pipe simultaneously.

The ProcessNextInstanceMessage method has also been refactored to use async I/O rather than running in a background thread, since WaitForConnectionAsync was a convenient way to allow cancelling its loop anyway.

Bonus Fix

The NamedPipeClientStream code was crashing with an index error when trying to re-summon an existing window without having any argument to pass to it, so that was also fixed.

Extra Note: Remaining Errors

Shutting down the application at the end of the sequence to relaunch as an administrator still ends up crashing consistently with a System.ArgumentOutOfRangeException (somewhere in System.Windows.Media.VisualCollection as part of an incredibly opaque traceback), which terminates the application, which is mostly okay/invisible since it is supposed to shut down at that point anyway. This isn't a new error; it was present before the changes in this PR, and I wasn't sure how to fix it, so I didn't touch it.

It seems to happen solely from the call to Application.Current.Shutdown() in the RunAsAdmin method, even when all the other logic around opening the new process is completely stripped out, so I don't think these changes affect it either way. It also triggers the error if you replace Application.Current.Shutdown() with a call to just Application.Current.MainWindow.Hide() (or .Close()), so as a guess, it seems like the error might be related to some hooks on window visibility.