Solved Is this a race condition...?
-
Hi all -
I'm concerned about a bit of code in a file transfer utility:
QObject::connect(m_sock, &QAbstractSocket::readyRead, this, &Widget::socketRead); ... void Widget::socketRead() { m_socketBytesRead = m_sock->read(m_buffIn, MAX_BUFF_SIZE - 1); } ... // in widget processing: bool b = m_sock->waitForReadyRead(3000); if (b) { if (m_socketBytesRead > 0) { // process m_buffIn } }
Could the in-widget test for m_socketBytesRead return before the read in the slot has completed?
Thanks...
-
I don't see any race here. You are waiting for 3 sec. If data does not come it will proceed as usual as no data has arrived.
-
@dheerendra
I don't think that is what he is thinking about. I presume he's asking whether he can guarantee thatsocketRead()
will have executed itsm_socketBytesRead = ...
beforem_sock->waitForReadyRead(3000);
returns and his code then executesif (m_socketBytesRead > 0) ...
. @mzimmers right? Based on absolutely nothing, I would guess that all signals/slots are fired beforewaitForReadyRead()
returns to caller, but it's a reasonable question. -
@JonB yeah, that's it exactly. I have two consumers of the signal, and it's not clear that the sequence of processing is predictable.
The more I think about this, the more I think my design sucks...
-
@mzimmers
One observation: aren't you Windows? Because if so you are encouraged to avoidwaitForReadyRead()
anyway, so would suggest redesign. At least you ought look at https://stackoverflow.com/a/44693193/489865 and act on it. -
I'm currently developing on Windows, but this app is going to be built to run on a Mac (for Apple the company, as a matter of fact). I've heard of this bug before, but I have yet to experience it. The alternative would be rather ugly: a timer, a wait, and then checking the return code. I'm tempted to try to live with it.
-
@mzimmers said in Is this a race condition...?:
The alternative would be rather ugly: a timer, a wait, and then checking the return code. I'm tempted to try to live with it.
Why would you have a timer and a wait for something that's asynchronous by design? Can't you just connect to
bytesAvailable()
and decide whether to read or not? -
@kshegunov my understanding is that bytesAvailable() is a function, not a signal, so I don't know how I'd connect to it.
I can see, though, that this is really a lousy design. I'm performing a file transfer synchronously from a slot. I think I need a separate thread to handle the socket IO, and a whole bunch of connections between it and my widget.
-
@mzimmers I don't see a need for a thread. Simply use asynchronous APIs instead of waiting.
-
@mzimmers
Far be it for me to risk interpreting what my learned colleague @kshegunov is getting at. But...As he & @jsulm are suggesting, why is it that your architecture requires
socketRead()
to set a variable and exit and your widget processing code waits for that withwaitForReadyRead()
? This is "synchronous" architecture. The encouraged pattern is for yourreadyRead()
slot to read the bytes and do whatever with them itself, perhaps raising a signal itself at the end to inform the widget. There is then no "waiting" for anything. This is "asynchronous" architecture. You can do all this in a single thread, no need for you to create one of your own. -
Is this a race condition?
It can't be a race condition unless you have multiple threads. Do your objects all live in the same thread?
@mzimmers said in Is this a race condition...?:
I think I need a separate thread to handle the socket IO
No, just move
// process m_buffIn
insidevoid Widget::socketRead()
-
@VRonin I only have one thread. Maybe "race condition" isn't the correct term, but I don't know whether I can depend on whether the socketRead() slot will update the member variable m_socketBytesRead before my test of that variable in the widget.
As background information, the feature I'm working on is transferring an Intel Hex file to an embedded device. The file's about 200K and the sockets are restricted to 1K, so there's a fair number of writes. After each write, I wait for an ACK before I send the next write.
-
@mzimmers said in Is this a race condition...?:
Maybe "race condition" isn't the correct term
It's not, you'd call that "slot interleaving" (due to the event loops being spun inside some slot), however this is beside the point.
After each write, I wait for an ACK before I send the next write.
What you describe is a stateful communication. You do that not by waiting but by managing a state machine. If this is TCP, then this
ACK
is really superfluous, the TCP stack does that for you behind the scenes. -
@kshegunov nice explanation...thanks. I've re-written the code so that the signal from the button opens the file to be transferred and performs an initial write. A slot catches the ACK and sends another chunk from the file. Much nicer now.
As an aside, the purpose of the ACK in this application isn't just to acknowledge receipt of a valid message, but to inform the sender that the receiver is ready for another message. The receiver is an 8-bit AVR device that is saving the file to a slow EEPROM, so the ACK is needed to prevent the sender from overwhelming the receiver's meager resources.