Wait until race condition fix #721
Conversation
Merge Upstream master
This fixes a race condition that resulted in the main RunLoop locking up, halting all tests indefinitely. This occured sometimes when `done()` was called asynchronously on a background thread. We are checking if the result for the test is complete (ie. `done()` has been called) on line 258, then running the `RunLoop`. If the background thread marks the test complete between the check on line 258 and the running of the `RunLoop` on line 260, it stops the `RunLoop` **prior** to the `RunLoop` being started instead of after. This means the `RunLoop` is never stopped and the test hangs forever. This ensures that the completion of the test and the stopping of the `RunLoop` is called serially on the main thread, which prevents the race condition. Unit test is included to verify the fix.
There was a problem hiding this comment.
This looks good to me, but I'm not in a position to merge this. I do have a question, though: do you know why XCTestExpectation doesn't work in your unit test? I wrote a simpler implementation that does fail without your patch, but I couldn't get it to succeed with your patch. It's not entirely surprising as what you're fixing is essentially a wholesale replacement for it, but it seems strange.
|
Hey Guys.. I am facing the "main loop unresponsive" issue and hence keen to know, when this PR is going to be merged.. Currently I have applied the fix locally to my machine but would be helpful if this PR goes in to keep things dynamic. |
|
@ikesyo would you be able to approve and merge this now? |
|
Could someone please make a minor release with this fix? |
|
I'm sorry for the delay, v8.0.6 has been released including the fix 🚀 |
This fixes issue #509. There was a race condition that resulted in the main RunLoop locking up, halting all tests indefinitely.
This occurred sometimes when
done()was called asynchronously on a background thread.We are checking if the result for the test is complete (ie.
done()has been called) on line 258, then running theRunLoop. If the background thread marks the test complete between the check on line 258 and the running of theRunLoopon line 260, it stops theRunLoopprior to theRunLoopbeing started instead of after. This means theRunLoopis never stopped and the test hangs forever. This ensures that the completion of the test and the stopping of theRunLoopis called serially on the main thread, which prevents the race condition.Unit test is included to verify the fix.
Checklist - While not every PR needs it, new features should consider this list: