Repository navigation
TestThread does not synchronize properly #1067
Description
PyScopeTest.TestThread does not properly synchronize access to res and th_cnt variables.
A lock needs to be added to ensure update calls are serialized.
We should increase the number of threads to make the issue stand out if regressed.
For more details see #1022 (comment)
Activity
fixed synchronization in PyScopeTest.TestThread as suggested in da97502
Fix synchronization in PyScopeTest.TestThread as suggested in da97502 (…
ddneilson commentedon Mar 6, 2020
Have you considered a race condition at the fixture level? The
PyScopeTestis not thread safe, in general. It is set up to have a single privatePyScopeat the class level that is created/disposed duringSetUp/Dispose, and referenced in each test. However, tests within the fixture can, and do, run in parallel.So,
pscan be modified mid-test for any test in this fixture.TestThreadis one of the longer tests in the fixture, so one might expect it to be more vulnerable to the data races expected of a shared state variable being replaced.Have you considered setting either setting
NonParallelizableon all of the tests in the fixture, or making the fixture itself threadsafe by making each test create its own, locally scoped,PyScope?edit: Correction,
TestThreadis the only test in this fixture that exits & reenters the GIL. So, it is the only one that can possibly seepsbeing mutated mid-test.lostmsu commentedon Mar 6, 2020
@ddneilson , can you point to the relevant documentation. AFAIK, by default NUnit runs all tests sequentially.
From https://github.com/nunit/docs/wiki/Framework-Parallel-Test-Execution :
ddneilson commentedon Mar 9, 2020
@lostmsu Interesting. We had seen this test fail as well, and traced our problem down to the fixture not being thread-safe; we were seeing the shared
PyScopebeing mutated. Didn't realize that the tests were intended to be run in serial.Fix synchronization in PyScopeTest.TestThread as suggested in da97502 (…