Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a few synchronization-related crashes #415

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Eta0
Copy link

@Eta0 Eta0 commented Mar 21, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant