2

I am trying to write a SingleApplication class that will only allow one instance of the program to be running. I am implementing this using QSharedMemory

The program works fine, unless I use a key with the value "42". Is there something wrong I am doing? Is this undefined behavior?

Main.cpp

int main(int argc, char *argv[])
{

    //QApplication a(argc, argv);
    SingleApplication a(argc, argv, "42"); //Does not work with '42'. Will work for any other value. 



    MainWindow w;
    w.show();


    return a.exec();
}

SingleApplication.h

class SingleApplication : public QApplication
{
    Q_OBJECT

public:
    SingleApplication(int &argc, char *argv[], const QString uniqueKey);

    bool alreadyExists() const{ return bAlreadyExists; }

    bool isMasterApp() const { return !alreadyExists(); }

    bool sendMessage(const QString &message);

public slots:
    //void checkForMessages();

signals:
    //void messageAvailable(const QStringList& messages);

private:
    bool bAlreadyExists;
    QSharedMemory sharedMemory;

};

SingleApplication.cpp

SingleApplication::SingleApplication(int &argc, char *argv[], const QString uniqueKey) : QApplication(argc, argv){

    sharedMemory.setKey(uniqueKey);

    //Create if one does not exist already
    if(sharedMemory.create(5000))
    {
        qDebug() << "Created!";

        bAlreadyExists = false;
    }
    else{
        if(sharedMemory.error() == QSharedMemory::AlreadyExists){
            qWarning() << "Program is already running!";
        }
    }
}
László Papp
  • 51,870
  • 39
  • 111
  • 135
Quaxton Hale
  • 2,460
  • 5
  • 40
  • 71
  • What is the concrete error ? – Oncaphillis Nov 02 '14 at 22:02
  • @Oncaphillis `sharedMemory.create()` simply returns false. Even for the first time the program is run. – Quaxton Hale Nov 02 '14 at 22:12
  • Could it be that the shared mem segment "42" is already in use by another process/thread ? I only now shm from linux and keys are just numbers which might be identical for different processes. There are helpers however that are supposed to help you keep them unique. Try a tool that lists the currenr shm segments. – Oncaphillis Nov 02 '14 at 22:15
  • @EngieOP: what does errorString() say? I do not get your use case and implementation though. – László Papp Nov 02 '14 at 22:30
  • 1
    **[42](http://en.wikipedia.org/wiki/Phrases_from_The_Hitchhiker%27s_Guide_to_the_Galaxy#The_number_42)** is globally allocated throughout the galaxy ;-) – Oncaphillis Nov 02 '14 at 22:45
  • @Oncaphillis That could be the case. I restarted my computer, and the problem no longer persists. HGTTG Ftw! :) @lpapp `"QSharedMemory::create: already exists"` – Quaxton Hale Nov 02 '14 at 23:36
  • @Oncaphillis: [42: The answer to life, the universe and everything](http://www.independent.co.uk/life-style/history/42-the-answer-to-life-the-universe-and-everything-2205734.html). – László Papp Nov 03 '14 at 07:39

2 Answers2

2

I propose you next solution. It's tested, but it's doesn't support sending messages between instances. And it resolves some bugs of your solution. Because it is not enough just to test memory. You need to guard a creation of shared memory.

RunGuard.h

#ifndef RUNGUARD_H
#define RUNGUARD_H

#include <QObject>
#include <QSharedMemory>
#include <QSystemSemaphore>


class RunGuard
{

public:
    RunGuard( const QString& key );
    ~RunGuard();

    bool isAnotherRunning();
    bool tryToRun();
    void release();

private:
    const QString key;
    const QString memLockKey;
    const QString sharedmemKey;

    QSharedMemory sharedMem;
    QSystemSemaphore memLock;

    Q_DISABLE_COPY( RunGuard )
};


#endif // RUNGUARD_H

RunGuard.cpp

#include "RunGuard.h"

#include <QCryptographicHash>


namespace
{

QString generateKeyHash( const QString& key, const QString& salt )
{
    QByteArray data;

    data.append( key.toUtf8() );
    data.append( salt.toUtf8() );
    data = QCryptographicHash::hash( data, QCryptographicHash::Sha1 ).toHex();

    return data;
}

}


RunGuard::RunGuard( const QString& key )
    : key( key )
    , memLockKey( generateKeyHash( key, "_memLockKey" ) )
    , sharedmemKey( generateKeyHash( key, "_sharedmemKey" ) )
    , sharedMem( sharedmemKey )
    , memLock( memLockKey, 1 )
{
        QSharedMemory fix( sharedmemKey );    // Fix for *nix: http://habrahabr.ru/post/173281/
        fix.attach();
}

RunGuard::~RunGuard()
{
    release();
}

bool RunGuard::isAnotherRunning()
{
    if ( sharedMem.isAttached() )
        return false;

    memLock.acquire();
    const bool isRunning = sharedMem.attach();
    if ( isRunning )
        sharedMem.detach();
    memLock.release();

    return isRunning;
}

bool RunGuard::tryToRun()
{
    if ( isAnotherRunning() )   // Extra check
        return false;

    memLock.acquire();
    const bool result = sharedMem.create( sizeof( quint64 ) );
    memLock.release();
    if ( !result )
    {
        release();
        return false;
    }

    return true;
}

void RunGuard::release()
{
    memLock.acquire();
    if ( sharedMem.isAttached() )
        sharedMem.detach();
    memLock.release();
}
Dmitry Sazonov
  • 8,801
  • 1
  • 35
  • 61
  • 1
    As far as I understood, the OP was interested in the "magic number" rather than the locking, and why that is not working. Locking will not solve the creation issue either, may just block or move on (depending on the lock functionality used), but in either case, the external process or thread will still keep the shared memory without the OP being to get a hold on it. – László Papp Nov 03 '14 at 11:28
  • @lpapp i this case he could use something like process explorer to see what real name has his shared memory, and check if it's busy. For me, original question sounds strange. I just point at mistake and propose to use hashes to prevent collisions. – Dmitry Sazonov Nov 03 '14 at 11:39
  • You could use the nativeKey() method for that, but I assume something in the background made that shared memory reserved unless the OP has made some silly mistakes. :) If it is the former so that something reserved the shared memory with that string, then it is tough luck unfortunately and there is not much that Qt can do about it. Also, the OP's concept will be broken on systems without shared memory. – László Papp Nov 03 '14 at 11:41
1

I would drop the whole own single application concept implemented by you from scratch personally. The qt-solutions repository has contained the QtSingleApplication class which was ported to Qt 5, too. You ought to be able to use that. There is little to no point in reinventing the wheel in my humble opinion.

Should you still insist on doing it on your own, while your idea seems to be a little strange at first about passing the key to the constructor and not to manage that inside the class transparently, this could be a workaround for your case to make the solution more robust:

SingleApplication::SingleApplication(int &argc, char *argv[], const QString uniqueKey) : QApplication(argc, argv)
{
    sharedMemory.setKey(uniqueKey);
    if (!sharedMemory.create(5000)) {
        while (sharedMemory.error() == QSharedMemory::AlreadyExists) {
            // Set a new key after some string manipulation
            // This just a silly example to have a placeholder here
            // Best would be to increment probably, and you could still use
            // a maximum number of iteration just in case.
            sharedMemory.setKey(sharedMemory.key() + QLatin1String("0"));
            // Try to create it again with the new key
            sharedMemory.create(5000);
        }
        if (sharedMemory.error() != QSharedMemory::NoError)
            qDebug() << "Could not create the shared memory:" << sharedMemory.errorString();
        else
        {
            qDebug() << "Created!";
            bAlreadyExists = false;
        }
    }

}

Disclaimer: this is just pseudo code and I have never tested it, actually, not even tried to compile it myself!

László Papp
  • 51,870
  • 39
  • 111
  • 135