Peter Petrov
Peter Petrov

Reputation: 1

Mutex not working properly

So this code is in program.cs and is supposed to check if connection is available and if there is another instance already running. If there is, message box which informs the user, and asks him if he is sure that he wants to open the application. Problem is next: I open the application, then open it again, message box shows but nothing happens. I repeat the proces and only after 4-5 time it works. Then, if I open again, it opens 2 instances.

static void Main()
    { 
        Application.EnableVisualStyles();
        Application.SetCompatibleTextRenderingDefault(false);
        SqlConnection con123 = new SqlConnection(con123.Metoda());
        Mutex mut = null;
        try
        {
            mut = Mutex.OpenExisting("Tray minimizer");
        }
        catch
        {

        }

        if (mut == null)
        {

            mut = new Mutex(true, "Tray minimizer");
            Application.Run(new Form1());

            //Tell GC not to destroy mutex until the application is running and
            //release the mutex when application exits.
            GC.KeepAlive(mut);
            mut.ReleaseMutex();
        }
        else
        {
            //The mutex existed so exit
            mut.Close();


                DialogResult result = MessageBox.Show("AApplication is already working!Do you want to reopen it?", "Caution!", MessageBoxButtons.OKCancel);

                if (result == DialogResult.OK)
                {


                    foreach (Process p in System.Diagnostics.Process.GetProcessesByName("Name of application"))
                    {
                        try
                        {

                            p.Kill();
                          //  p.WaitForExit(); // possibly with a timeout

                            Application.Run(new Form1());
                        }
                        catch (Win32Exception winException)
                        {
                            // process was terminating or can't be terminated - deal with it
                        }
                        catch (InvalidOperationException invalidException)
                        {
                            // process has already exited - might be able to let this one go
                        }
                    }

                }
                //if (result == DialogResult.Cancel)
                //{


                //}

            }

            try
            {
                con123.Open();
                con123.Close();
            }
            catch
            {

                MessageBox.Show("Cant connect to server!!!", "Error!");
                Application.Exit();
            }

Upvotes: 0

Views: 3274

Answers (3)

Peter Ritchie
Peter Ritchie

Reputation: 35869

I would do something more like:

bool mutexCreated = true;
using (Mutex mutex = new Mutex(true, "eCS", out mutexCreated))
{
    if (mutexCreated)
    {
        Application.EnableVisualStyles();
        Application.SetCompatibleTextRenderingDefault(false);
        SqlConnection con123 = new SqlConnection(con123.Metoda());

        Application.Run(new Form1());
    }
    else
    {
        DialogResult result = 
            MessageBox.Show("AApplication is already working!Do you want to reopen it?", "Caution!",
                                                MessageBoxButtons.OKCancel);

        if (result == DialogResult.OK)
        {
            foreach (Process p in System.Diagnostics.Process.GetProcessesByName("Name of application"))
            {
                try
                {
                    p.Kill();

                    Application.Run(new Form1());
                }
                catch (Win32Exception winException)
                {
                    // process was terminating or can't be terminated - deal with it
                }
                catch (InvalidOperationException invalidException)
                {
                    // process has already exited - might be able to let this one go
                }
            }

        }
    }

    try
    {
        con123.Open();
        con123.Close();
    }
    catch
    {

        MessageBox.Show("Cant connect to server!!!", "Error!");
        Application.Exit();
    }
}

The problem with your version is that the mutex might get collected at an inappropriate time.

Upvotes: 2

Tawnos
Tawnos

Reputation: 1887

Does this solve your problem? Rather than using the OpenExisting, which has no documentation guarantee of returning null when it fails, I used the Mutex constructor that takes an out bool to determine if the mutex was created or if it already exists. Launching the application and doing anything with it (partially guessing based on what I see) is moved below everything related to creating the mutex or closing existing instances.

Steps are now as follows:

  1. Initialize the runApp variable to true
  2. Try to create the Mutex
  3. Check if the Mutex was created (doesn't already exist)
    • If the Mutex wasn't created, it already exists
      • Ask user if they want to exit, wait for the mutex to be available (indicating completion of forcibly exiting the existing application instance)
      • If they don't want to exit, set runApp to false
  4. Check to see if the runApp flag is still true

    • If it is true, run the application. After that returns (the form exits), try connection

      Note, this may have a bug, I don't know if you intend to block as you are in the app.

  5. Release the mutex

    Application.EnableVisualStyles();
    Application.SetCompatibleTextRenderingDefault(false);
    SqlConnection con123 = new SqlConnection(con123.Metoda());
    string ProgramName = "Tray minimizer.exe";
    bool mutCreated = false;
    Mutex mut = new Mutex(true, ProgramName, out mutCreated);
    bool runApp = true;
    if (!mutCreated)
    {
        DialogResult result = MessageBox.Show("Application is already working! Do you want to reopen it?", "Caution!", MessageBoxButtons.OKCancel);
    
        if (result == DialogResult.OK)
        {
            foreach (Process p in System.Diagnostics.Process.GetProcessesByName(ProgramName))
            {
                try
                {
                    p.Kill();
                }
                catch { }
            }
            mut.WaitOne(); // Wait for ownership of the mutex to be released when the OS cleans up after the process being killed
        }
        else
        {
            runApp = false;
        }
    }
    
    if (runApp)
    {
        Application.Run(new Form1());
    
        try
        {
            con123.Open();
            con123.Close();
        }
        catch
        {
    
            MessageBox.Show("Cant connect to server!!!", "Error!");
            Application.Exit();
        }
    
        mut.ReleaseMutex();
    }
    

Upvotes: 0

Brandon
Brandon

Reputation: 39212

  1. Once you end up in the "mutex exists" path, you never ever release the mutex. You just kill any other processes and launch your app again, but never provide a way to ever release the mutex when your new app ends.

  2. You are launching your app within the foreach (Process) loop, which means if there are multiple processes already running (perhaps all with the message box), you will launch your app for each one.

  3. This also means you will not launch your app if you don't actually find another process to kill.

Your else case should look something like this pseudo-code:

dialogResult = MessageBox(...);
if (dialogResult == OK)
{
    foreach (var p in ...)
    {
        // Todo: make sure you do not kill the current process!
        p.Kill();
    }

    // now run the application
    Application.Run(new Form1());
    // now release the mutex
    mut.Release();
}
else
{
    mut.Close();
}

That pseudo code still needs some exception handling to make sure the mutex is released properly if an exception occurs.

Upvotes: 0

Related Questions