Is my multithread application proper?
-
Hello. I designed an application which contains multithtread, and messaging over TCP in a 1-5 ms time period. But I'm new to multithread stuffs. I would appreciate if you could look over it if it is a suitable use.
-Getting received data:
/*TcpServer class source file*/ TcpServer::TcpServer { MessageParser *parser = new MessageParser; parser->moveToThread(&m_parserThread); connect(this, SIGNAL(parseSignal(MessagePkg )), parser, SLOT(parseMessage(MessagePkg ))); } /*ReadyRead signal of my socket object is connected to receiveProcess method*/ TcpServer::receiveProcess() { quint16 bytes_available = m_socket->readAll(); /* Some stuffs to push the data inside of bytes_available to my message package*/ ... /* Switching to thread process to parsing message package*/ m_parserThread.start(); emit parseSignal(msg_pkg); }
In parseMessage method, I'm only assigning the appropriate bytes to my global variables which are shown in the Gui side. At the other side, I'm refreshing the variables on the screen with a timer which has 500ms time period. I guess it is clear to understand. What do you think about my design, is it correct approach for multithreading.
What if my parseMessage(MessagePkg) method like this:
MessageParser::parseMessage(MessagePkg) { /* Some parsing stuffs*/ ... emit doLogSignal(MessagePkg); /* For logging message package */ }
Normally, I think I'm starting the thread after every package is ready in Tcp server, and thread is finishing '}' of parseMessage method, right? What if I emit a signal for logging, what will be lifetime of the thread? Same way, Will the thread be finished at '}' of parseMessage, after the log slot is completed?
Thanks a lot.
-
You should and must not start the thread during receive. Start them in the ctor. Then it's eventloop is running and the slot can be executed.
Stop the thread in your dtor later on.Also readAll() is probably not what you want to use.
-
Thank you. But, my TcpServer object alive along the program lifecycle. So will it enter the dtor, if I want to stop the thread in the dtor? Will the thread not finish with the end of the my thread slot method? Do I need m_parserThread->quit to finish the thread.
Am I starting during receive? I was thinking that I'm starting thread after receive operation is finished?
Another thing, what should I use instead of readAll(). Could you open a little bit more, please? I could not get it exactly.
-
@eoreyg said in Is my multithread application proper?:
ill the thread not finish with the end of the my thread slot method?
No, at least not until you somehow stop them which I doubt you do. Please read the docs about QThread.
wrt: readAll() - readyRead() is called every time bytes are available. Therefore you have to parse your stream to find your packet inside the stream.
-
@eoreyg said in Is my multithread application proper?:
Normally, I think I'm starting the thread after every package is ready in Tcp server, and thread is finishing '}' of parseMessage method, right? What if I emit a signal for logging, what will be lifetime of the thread? Same way, Will the thread be finished at '}' of parseMessage, after the log slot is completed?
Normally you start/stop the thread using signals & slots.
Will the thread not finish with the end of the my thread slot method? Do I need m_parserThread->quit to finish the thread.
If your
MessageParser
is done with its work (in your case, when you stop your TCP connection), you connect for exampleMessageParser::done
withQThread::quit
.If you want to stop it earlier and start a new thread, you have to do it.
-
@Pl45m4 I don't see why the thread should be stopped here at all. The code doesn't show any hints for it.
-
@Christian-Ehrlicher said in Is my multithread application proper?:
I don't see why the thread should be stopped here at all
Yes, but as far as I understand, @eoreyg wants to start/stop the thread after the processing of every result (for whatever reason)...
-
@Pl45m4 But then the code above will not work since the signal may not be executed (since the thread or better the eventloop may not run yet then). It's just wrong and useless.
-
@eoreyg said in Is my multithread application proper?:
still bad approach?
Definitely :)
Why you want to restart the thread every time?
Start your thread in your constructor or while you initalize everything, pass your worker to your thread and then just let it keep running. You can communicate with your worker and thread using signals&slots, as I said before (they are asynchronous). Your log-Signal looks fine.