-
-
Notifications
You must be signed in to change notification settings - Fork 219
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Multithreading race. It is not a solution. #3
Comments
That is not possible, since the filesystem won't allow two processes to bind to the same local socket / file. |
Besides, if there are no running connections, the first process will bind to the file almost instantaneous and the second will manage to connect. The timeout was there since your system might have been under heavy load. |
maybe. but where is a error checking for that? 2015-05-07 17:07 GMT+03:00 Itay Grudev notifications@github.com:
|
auto server1 = new QLocalServer(); auto server2 = new QLocalServer(); ...... not blocking. I may reproduce this code many hundred copies. |
Yes, because in your code each subsequent call removes the previous server. Now add the check for connection. QLocalServer *server1 = new QLocalServer();
server1->removeServer("111");
server1->listen("111");
QLocalServer *server2;
QLocalSocket *socket = new QLocalSocket();
socket->connectToServer("111");
if(socket->waitForConnected(1000)){
server2 = new QLocalServer();
server2->removeServer("111");
server2->listen("111");
} With the check it won't work. The filesystem provides a lock feature which is a mutex by itself. Therefore no race would occur since the file is locked and accepting connections. The only reason a connection is not established within 1 second is that the "old server" process has crashed. In which case a new instance would be necessary. |
run 2 threads (processes) in one code: // Initial state = no running servers QLocalSocket *socket1 = new QLocalSocket(); // thread 1 socket1->connectToServer("111"); // thread 1 if(!socket1->waitForConnected(1000)) // thread 1 - fault wait if(!socket2->waitForConnected(1000)) // thread2 - fault wait And now we have 2 (or more) threads that execute 'code1': /-------------- thead 1 ---------------------------- /-------------- thead 2 ---------------------------- and of course we have 2 or more copies of the program running. your solution is not working. |
@heretic13 were you able to reproduce the issue? |
I was able to. I will review the issue more carefully and think of a solution. Thanks! |
For reference, this is the code I used: #include <QLocalServer>
#include <QLocalSocket>
#include <QThread>
#include <cstdio>
#define SERVER_NAME "111"
struct SocketOrServer : public QThread {
public:
SocketOrServer(int threadNum) : QThread(), threadNum(threadNum) { }
private:
void run()
{
printf("connecting\n");
socket = new QLocalSocket();
socket->connectToServer(SERVER_NAME);
if( ! socket->waitForConnected(1000)) {
delete socket;
printf("creating server on thread %d\n", threadNum);
server = new QLocalServer();
server->removeServer(SERVER_NAME);
server->listen(SERVER_NAME);
}
}
int threadNum;
QLocalSocket *socket;
QLocalServer *server;
} client1(1), client2(2);
int main(int argc, char *argv[])
{
client1.start();
client2.start();
client1.wait();
client2.wait();
} P.S. Though the class was intended to avoid a user starting your app twice, not a computer. |
@itay-grudev but with the "removeServer" lines? Because with them it's easy to reproduce the issue. Also it may be only Windows platform issue as the docs state: On Windows two local servers can listen to the same pipe at the same time, but any connections will go to one of the server. |
@kklimek I am actually testing it under Unix, the code is above. |
My goal was to find a singleton implementation for qt. I also check here - http://habrahabr.ru/post/173281/ but author have a mistake. They make attach memory then create - possible multithreading race. Relly simple singleton - use QSharedMemory mem1("111"); But author say that app-crashed on linux not cleanup QSharedMemory. I think the truth (for singleton) is somewhere in the middle. |
This bug is related to issue #3. I expect to push a solution soon
@heretic13 you are correct. I also thought that the solution is somewhere in the middle. I will investigate it and push a fix as soon as possible. |
I've came up with solution like this:
|
This bug is related to issue #3. I expect to push a solution soon
This bug is related to issue #3. I expect to push a solution soon
|
"QSharedMemory is not thread safe." Provide an example. I must see it. |
I have an idea. I can get around that. (Or it might have been that I misused |
This will work. I have a working prototype. I will integrate it in the library and I will push a commit later today with a fix. ; ) |
Run 2 or more processes.
They pseudo-paralell execute code
"socket->waitForConnected(1000)"
then they all go to the branch else
{
// If the connection is insuccessful, this is the main process
// So we create a Local Server
server = new QLocalServer();
server->removeServer(serverName);
server->listen(serverName);
QObject::connect(server, SIGNAL(newConnection()), this, SLOT(slotConnectionEstablished()));
}
and now we have 2 or more running processes;
The text was updated successfully, but these errors were encountered: