6
\$\begingroup\$

I've inherited an existing application which is a WPF implementation of toast notifications. This application is installed to multiple client PCs across the estate and, once installed, runs in the background. The notifier then works regardless of who is logged onto the machine. One of the improvements I have been asked to make is to prevent the application from being started more than once.

I am considering using a mutex to do this. I've implemented a solution that functionally works at startup using inspiration from MSDN and Stack Overflow but would like to identify any potential weaknesses or scenarios which could break the code. Ultimately, I don't want some crash to occur that might fail to dispose the mutex or leave the application unable to start, although I think I have catered as best possible.

public partial class MainWindow : Window
{
    // unique id for application mutex
    private static string mutexId = string.Format("Local\\{{{0}}}", "1109F104-B4B4-4ED1-920C-F4D8EFE9E833");

    private static string applicationName = "My App";

    private bool hasHandle;

    private Mutex mutex;

    /// <summary>
    /// Initializes a new instance of the MainWindow class.
    /// N.B. Inspiration for this implementation sourced from:
    /// https://stackoverflow.com/questions/229565/what-is-a-good-pattern-for-using-a-global-mutex-in-c/229567
    /// </summary>
    public MainWindow()
    {
        this.mutex = new Mutex(false, mutexId);

        // Allow all users of the system to access the mutex with full control.
        var allowEveryoneRule = new MutexAccessRule(new SecurityIdentifier(WellKnownSidType.WorldSid, null),
            MutexRights.FullControl, AccessControlType.Allow);
        var securitySettings = new MutexSecurity();
        securitySettings.AddAccessRule(allowEveryoneRule);
        mutex.SetAccessControl(securitySettings);

        try
        {
            try
            {
                hasHandle = this.mutex.WaitOne(1000, false);
                if (!hasHandle)
                {
                    MessageBox.Show("Application is already running.", applicationName);
                    Application.Current.Shutdown();
                }
            }
            catch (AbandonedMutexException)
            {
                // Log the fact the mutex was abandoned in another process, it will still get aquired 
                hasHandle = true;
            }

            // Perform your work here. 
            InitializeComponent();
            Closing += (s, e) => Shutdown(); 
        }
        catch (Exception e)
        {
            //If InitializeComponent throws an error, gracefully cleanup the application.
            Shutdown();
        }

    }

    private void Shutdown()
    {
        ViewModelLocator.Cleanup();

        // Release the mutex if a handle was aquired while the application was running.
        if (this.hasHandle)
        {
            this.mutex.ReleaseMutex();
        }

        this.mutex.Dispose();
    }
}

For brevity I've omitted using statements and namespace declarations, as well as the .xaml files and existing solution, as I want to focus on this new improvement, which is self-contained. I'm also conscious of trying to keep startup and shutdown as simple as possible, so would happily consider improvements to remove unneeded code.

Finally, I have seen this similar question and would be happy with an answer to use this example if this is the best way of meeting the requirement.

\$\endgroup\$
9
\$\begingroup\$

Thing that make me more unhappy is mixing UI logic of your MainWindow class with this single instance logic that really belongs to process (and then Application).

First thing I'd do is to introduce an explicit Main() method (follow any tutorial out there) and leave MainWindow to deal with Window things. I'd then introduce a SingleInstanceApplicationLock class (pick a better name, nothing came to my mind in this moment). I'd like to use it like this:

public static void Main()
{
    using (var appLock = new SingleInstanceApplicationLock())
    {
        if (!appLock.TryAcquireExclusiveLock())
            return;

        var app = new App();
        app.InitializeComponent();
        app.Run();
    }
}

SingleInstanceApplicationLock will then be an internal sealed class implementing IDisposable (where you release resources you acquired). AcquireExclusiveLock() may even be lazy-load the mutex (I'd avoid to complicate things if unneeded but I like even less to put too much complex logic in constructors). Proof of concept:

sealed class SingleInstanceApplicationLock : IDisposable
{
    ~SingleInstanceApplicationLock()
    {
        Dispose(false);
    }

    void IDisposable.Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    public bool TryAcquireExclusiveLock()
    {
        try
        {
            if (!_mutex.WaitOne(1000, false))
                return false;
        }
        catch (AbandonedMutexException)
        {
            // Abandoned mutex, just log? Multiple instances
            // may be executed in this condition...
        }

        return _hasAcquiredExclusiveLock = true;
    }

    private const string MutexId = @"Local\{1109F104-B4B4-4ED1-920C-F4D8EFE9E833}";
    private readonly Mutex _mutex = CreateMutex();
    private bool _hasAcquiredExclusiveLock, _disposed;

    private void Dispose(bool disposing)
    {
        if (disposing && !_disposed && _mutex != null)
        {
            try
            {
                if (_hasAcquiredExclusiveLock)
                    _mutex.ReleaseMutex();

               _mutex.Dispose();
            }
            finally
            {
                _disposed = true;
            }
        }
    }

    private static Mutex CreateMutex()
    {
        var sid = new SecurityIdentifier(WellKnownSidType.WorldSid, null);
        var allowEveryoneRule = new MutexAccessRule(sid,
            MutexRights.FullControl, AccessControlType.Allow);

        var securitySettings = new MutexSecurity();
        securitySettings.AddAccessRule(allowEveryoneRule);

        var mutex = new Mutex(false, MutexId);
        mutex.SetAccessControl(securitySettings);

        return mutex;   
    }
}
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.