0

I'm working on an app that should only ever have one instance running. I've just added a class that's meant to represent the "single instance acquisition", where if the executable is seen as already running, it passes the command line parameters to a listening method on the other side. It uses a Win32 mutex as well as a boost message_queue.

InstanceManager::InstanceManager()
    {
        LPCSTR versionCstr = "MyApp";
        myMutex = OpenMutex(MUTEX_ALL_ACCESS, FALSE, versionCstr);

        if (!myMutex && GetLastError() == ERROR_FILE_NOT_FOUND)
        {
            // This mutex did not already exist, so we are the first to the plate. Create mutex, message queue
            myMutex = CreateMutex(0, TRUE, versionCstr);
            message_queue::remove("MyApp");
            messageQueue.reset(new message_queue(create_only, "MyApp", 5, 512));
            receiveThread.reset(new boost::thread(bind(&InstanceManager::receiveThread_Internal, this)));
        }
        else if (!myMutex)
        {
            // Could not create mutex, but can't tell why
            throw exception("Unexpected error creating mutex - " + GetLastError());
        }
        else
        {
            // Mutex already existed. We are the second instance of this program. (Don't do anything just
            // yet - send method is outside)
            messageQueue.reset(new message_queue(open_only, "MyApp"));
        }
    }

It seems like about 3 out of 5 times, this works correctly. It enters the first if block if this is the first exe instance. It enters the else block if this is the second. However, sometimes it reaches the constructor for message_queue and encounters an interprocess_exception, with detail: "The system cannot find the file specified." At this time, there is another instance of the app running, and it should be listening for messages (that's what receiveThread does).

Given that I can't even tell why message_queue would be working with files (I really hope it's not writing to disk to "simplify") I'm not really sure what to look at with this.

There's more to my code, but it doesn't reach it, so I feel like that couldn't ever be relevant.

Katana314
  • 8,429
  • 2
  • 28
  • 36

1 Answers1

0

The problem is that InstanceManager constructor and if statement can be entered by more than one application at the same time if mutex file is not created yet. You can lock access to InstanceManager per one application using boost's named_mutex. Also message queue would need to be removed by last process (you will need to track somehow which process is last one quitting, for example by using shared memory).

Better implementation could look like this:

InstanceManager::InstanceManager()
{
    using boost::interprocess;

    named_mutex mutex(open_or_create, "my_named_mutex");
    scoped_lock<named_mutex> lock(mutex);

    try
    {
        messageQueue.reset(new message_queue(open_only, "MyApp"));
        return;
    }
    catch(interprocess_exception &ex)
    {
        // OK, this is the first instance so we need to create the message queue first
    }

    messageQueue.reset(new message_queue(create_only, "MyApp", 5, 512));
    receiveThread.reset(new boost::thread(bind(&InstanceManager::receiveThread_Internal, this)));
}

InstanceManager::~InstanceManager()
{
// last process removes the queue
    message_queue::remove("MyApp");


    named_mutex::remove("my_named_mutex");
}
doqtor
  • 8,414
  • 2
  • 20
  • 36
  • The time between OpenMutex and CreateMutex is something on the order of nanoseconds. While I agree that could be a future issue, that's almost certainly not the current issue - the second instance is started at a user-defined moment in time at least several seconds away. I also don't like relying on exceptions for baseline behavior, but if that's the only way of finding out whether the messageQueue has been created, I can look into it. – Katana314 Jul 29 '15 at 13:36