[QC-1038] fix Race condition in Service Discovery #2024
Conversation
|
@awegrzyn could you review too ? |
|
Seems ok, but what about simply running |
|
@awegrzyn I thought about it but we call |
|
Ah okay, understood, but I have a impression that we're getting back to original problem => QC main thread stuck because of waiting for port selection. @knopers8 Could you confirm? |
|
The original problem was different. The port selection routine was selecting a port by binding a port in range until succeeded, then releasing it and letting the new thread bind it again. In the time gap between releasing the port and binding it again, sometimes another process was binding it. I will review the PR tomorrow |
|
Ah ok, so if you don't mind to wait in main thread for port selection that it's fine. |
|
If I understand correctly what I did, we wait in the main thread for the port selection (that should not take really long) and if none is found it will release the main thread any ways. I did some tests and it seemed to work. Thank you Piotr for reviewing it. Ideally I would like to have it in today's release but if there is a logical flaw we will wait. |
| // Wait until port is set by the thread | ||
| { | ||
| std::unique_lock<std::mutex> lk(mHealthPortMutex); | ||
| mHealthPortCV.wait(lk, [this] { return mHealthPortAssigned == true; }); |
There was a problem hiding this comment.
Maybe I misunderstood how condition_variable works, but won't it block the second time you call _register? I am not sure if it requires the other thread to call notify_one() for each wait.
There was a problem hiding this comment.
OK, I wrote a little reproducer and it seems it my concern is not valid, it seems that notify_one() unblocks the main thread forever, as long as the predicate is provided in wait.
There was a problem hiding this comment.
as you noted, as long as the predicate is true, it will let the main thread continue.
I used a mutex and a condition variable with a flag to know that the port has been assigned. Perhaps there is a better way ?