PDA

View Full Version : QSharedMemory locking-Question



engelhardt
24th May 2011, 18:35
Hello guys,

the locking with the QSharedMemory class behaves very strange with me.

Let me demonstrate:


// T1
#include <QSharedMemory>

int main() {
QSharedMemory qs("TEST");
qs.create(10);
qs.attach();

qs.lock();
while (1);
}
// T1

and


// T2
#include <QSharedMemory>
#include <iostream>

int main() {
QSharedMemory qs("TEST");
qs.create(10);
qs.attach();

qs.lock();
std::cout << "Here?\n";
}
// T2


Now I run T1 first:


$ ./t1


and some seconds later I run T2 and I get:


$ ./t2
Here?
QSharedMemory::lock: already locked


Huh? Here should never be reached, if I read the documentation correctly. How is this possible? I don't understand this behavior and it makes the whole locking function pointless, doesn't it?

Can anybody help me out?

Best regards!

squidge
24th May 2011, 18:59
Those functions return a bool saying whether or not they were successfull, you might want to check that and call error() if one of them returns false.

Also, create does an attach for you, and you can't create from two processes with the same key.

kaiben
4th February 2015, 07:27
Hi all,

sorry squidge, but your comment or hint saying that "create does an attach for you, and you can't create from two processes with the same key." is not correct.
A create operation do an attach only if the creation was successful. If not the creation process fails. One reason upon other cases can be that the shared variable already exist.
In that case you can try to attach to them. So even if the example below is not complete it seems to use the Qt interface as specified by the documentation.
I have same problem on Linux. Same code works fine in a Windows environment.

I found a QTBug-1036 https://bugreports.qt.io/browse/QTBUG-10364 but it seems not being handled by Digia.

So my question here is ... what can I do to work around this issue on a Linux platform?

Please help.

Bye
Kai

jefftee
4th February 2015, 07:55
You are still ignoring the return values from QSharedMemory::create and QSharedMemory::attach.

What are the return values from those calls?

kaiben
4th February 2015, 12:04
Hi,

I did a lot of testing today. I can now reproduce the error case in a Linux system.
Even if I do not undertstand the reason the fix is - as documented in the thread -
to invert the function calls create and attach.

Please use the attached code to reproduce both cases
Usage:
- Error case undef or comment #define REVERSE
- working case define or uncomment #define REVERSE
After compilation launch multiple instances of it and try different scenarios.
Hope this help!

Bye
Kai



#include <QSharedMemory>
#include <QDebug>



bool shmCreate( QSharedMemory & qs )
{
qDebug( ) << "About to create shared memory. Press key to execute.";
getchar( );

qDebug( ) << "\nShared memory creation in progress...\n";
if( qs.create( 10 ) )
{
qDebug( ) << "Shared memory successfully created.\n";
return true;
}
else
{
qDebug( ) << "Shared memory creation failed.\n";
qDebug( ) << qs.errorString( ) << "\n";
return false;
}
}



bool shmAttach( QSharedMemory & qs )
{
qDebug( ) << "About to attach to shared memory. Press key to execute.";
getchar( );

qDebug( ) << "\nAttaching to shared memory...\n";
if( qs.attach( ) )
{
qDebug( ) << "Shared memory successfully attached.\n";
return true;
}
else
{
qDebug( ) << "Attaching to shared memory failed.\n";
qDebug( ) << qs.errorString( ) << "\n";
return false;
}
}



void shmLock( QSharedMemory & qs )
{
qDebug( ) << "About to lock shared memory. Press key to execute.\n";
getchar( );

qDebug( ) << "\nLocking shared memory...\n";
if( qs.lock( ) )
{
qDebug( ) << "Shared memory successfully locked.\n";
}
else
{
qDebug( ) << "Locking shared memory failed.\n";
qDebug( ) << qs.errorString( ) << "\n";
}
}



void shmUnlock( QSharedMemory & qs )
{
qDebug( ) << "About to unlock shared memory. Press key to execute.";
getchar( );

qDebug( ) << "\nUnocking shared memory...\n";
if( qs.unlock( ) )
{
qDebug( ) << "Shared memory successfully unlocked.\n";
}
else
{
qDebug( ) << "\nUnlocking shared memory failed.\n";
qDebug( ) << qs.errorString( ) << "\n";
}
}



void shmDetach( QSharedMemory & qs )
{
qDebug( ) << "About to detach from shared memory. Press key to execute.";
getchar( );

qDebug( ) << "\nDetaching from shared memory...\n";
if( qs.detach( ) )
{
qDebug( ) << "Successfully detached from shared memory.\n";

}
else
{
qDebug( ) << "\nDetaching from shared memory failed.\n";
qDebug( ) << qs.errorString( ) << "\n";
}
}


#define REVERSE REVERSE


int main(int argc, char *argv[])
{
QSharedMemory qs( "TEST" );

while( true )
{
#ifdef REVERSE
// Attach, then create.
if( shmAttach( qs ) ) break;
if( shmCreate( qs ) ) break;
#else
// Create, then attach.
if( shmCreate( qs ) ) break;
if( shmAttach( qs ) ) break;
#endif
if(qs.error() == QSharedMemory::AlreadyExists || qs.error() == QSharedMemory::NotFound)
qDebug( ) << "\nRetry again ...";
else{
qDebug( ) << "\nError occurred. Cannot continue. Terminate application.\n";
return 0;
}
}

shmLock ( qs );
qDebug( ) << "\nDo critical stuff...\n";
shmUnlock( qs );
shmDetach( qs );

return 0;
}

jefftee
5th February 2015, 02:52
Even if I do not undertstand the reason the fix is - as documented in the thread -
to invert the function calls create and attach.

Seems to make sense to me. When performed in the "reverse" order as you refer to it, you attempt to attach to the shared memory segment. If that is not successful, then you attempt to create it, all the while ignoring why the attach failed because you did not check the error after attach returned false.

If the attach failed because it does not exist, then you will successfully create (and implicitly attach) to the share memory segment by calling create.

To handle properly IMHO, you should try to attach to the shared memory segment. If attach returns false and the error return value is QSharedMemory::NotFound, then and only then would I attempt to create the shared memory segment.

This approach works if it's the first instance of your program, the 2nd, 3rd, 4th, etc.

In your very first attempt, you called create, attach, lock in that order. Worked great for the 1st instance, but 2nd and subsequent instances, create would fail because it already exits, then you would attempt to attach and lock. Reading the doc for QSharedMemory::lock(), it specifically says:



This is a semaphore that locks the shared memory segment for access by this process and returns true. If another process has locked the segment, this function blocks until the lock is released. Then it acquires the lock and returns true. If this function returns false, it means that you have ignored a false return from create() or attach(), that you have set the key with setNativeKey() or that QSystemSemaphore::acquire() failed due to an unknown system error.

Says right there that you attempted to lock after ignoring a false return from either lock or attach and your 2nd instance and beyond would *always* receive a false return code from the create attempt. Based on that I'm not surprised it doesn't work, so I'm more surprised that it works on Windows than you are surprised it doesn't work on Linux... :)

Develop good programming habits and learn to not ignore return values from functions. You will save yourself hours and hours of frustration not understanding why something isn't working. Especially as something as simple as using the error information provided by the function, it's trying to tell you what the problem is!

Good luck

kaiben
6th February 2015, 09:32
Hi jthomps,

Ok maybe the following testcases will convince you that something is wrong with the Qt implementation.

Please take a look at the attached picture where two interaction flows between process A and B are
described. I did the test with the source code previously send to this thread.

10944

Please tell me what do you thing about the second case? No errors are returned from the interface, so
it is impossible to handle this case at all.
Again it is strange to me that the reverse order (attach then create) seeems to work in all cases.
Maybe I did not catch all cases during my testing. But it should not be my task to do that....

Its up to you to decide what to do next...
For me it is important to have a hopefully errorfree workaround on all platforms.
And if doing first an "attach" and then a "create" let the software works as expected.... I can live with it.
Furthermore it is important to me to complete this thread with all information I have, so that other people do not fall in this trap.

Thank you for your fast help to find a solution.

Bye
Kai

jefftee
6th February 2015, 22:31
I did explain why I thought the reverse order works and I do agree that the create/attach/lock approach does indeed appear to allow both processes to lock the shared memory at the same time. I believe the documentation could be improved to state which order (if relevant) the create or attach should be performed. This could be by design or a bug in QSharedMemory code.

I don't agree that it's not your task to catch all cases in your testing though. As a developer, you have to code defensively and anticipate errors and properly handle errors or else the impact on your application will be unpredictable. The first example you posted in this thread had zero error checking.

I'm also not sure what you meant by "its [sic] up to me to decide what to do next". I am not working on anything that uses or requires QSharedMemory, so how you handle this is up to you, not me. I was only trying to help and if you don't believe I have or can help, I'm happy to stop trying... :)

That said, here's what I would do if I were you:



Write/test the code in the "reverse" order because so far in your testing, I believe you agree that this works on at least the two platforms you have tested (Windows and Linux). I have confirmed this approach also works on the Mac platform.


If you believe you have found a bug in QSharedMemory, I'd suggest that you submit a bug report ("https://bugreports.qt.io/login.jsp).


Edit: found this bug (https://bugreports.qt.io/browse/QTBUG-10364) that may be relevant to what you are seeing.

Good luck

kaiben
9th February 2015, 08:07
Hi jthomps,

Thank you for your answer again and I'm sorry. I supposed that you are involved in the development of the Qt Framework.
With "you" I mean the digia company.

I completely agree with you regarding development of software.
With "testing all cases" I mean... consider all possible interaction/interference between the processes running concurrently when using the QSharedmemory interface.

And finally I'm not the creator of this thread. Engelhardt did this some years ago and he posted the simple code to indicate the issue.


Bye
Kai

wysota
9th February 2015, 08:28
In my opinion both these approaches are incorrect :) Regardless of the order of calls both of them may fail. The create-attach order is wrong as if create fails (the key exists) and you call attach, the block may be destroyed and your attach is going to fail leaving you with nothing. If you reverse the order and call attach first and then create, a create call from another process might fit between your calls and create will fail as well. The only solution I can see other than using a semaphore for synchronizing creation of the segment is to use a while loop in which you will be calling create and attach. And I think in this case the order of calls will not matter. Correct me if I'm wrong.

kaiben
9th February 2015, 09:17
Hi wysota,

If you look at the code I posted in this thread, then the create/attach, attach/create functions are within a while loop.
http://www.qtcentre.org/threads/41868-QSharedMemory-locking-Question?p=272341#post272341

Bye
Kai

wysota
9th February 2015, 09:57
If you look at the code I posted in this thread, then the create/attach, attach/create functions are within a while loop.

Yeah, and that's ok, if you ask me. And the order of calls doesn't matter. If both processes can enter the critical section at this point then there is simply a bug in the lock implementation. You can work around this by using your own semaphore.