Repository navigation
tests: fix three stop and clock races (isp1040, ioc3, ioc3_eth) - #380
Merged
Merged
Conversation
The tests drive the adapter on a fake clock. While that clock is in use, the ring thread reads it with the state locked and then waits on the condition variable with no timeout, because only the test can move it. Rig::clock stored the new time and notified without taking the state lock. A move that landed after the thread read the clock and before it waited was lost: the thread slept through the time that ended its selection, and the test's wait_idle then waited for that selection forever. Run alone, this test hung 6 times in 1000: a_reset_naming_a_missing_bus_leaves_a_waiting_selection_and_is_noted with the ring thread in wait_selection and the test in wait_idle, both parked on the same condition variable. The clock now moves through Core::set_test_clock, which takes the state lock first. Guests never use the fake clock: on the host's clock each wait for a time to pass has a timeout, and the waits without one are for state that changes under the lock, so guests could not hang this way. The new test moves the clock past 2000 selection timeouts, spacing the two moves across the thread's wake-up. Without the lock it fails within its 5 s limit in 91 of 100 runs.
Contributor
|
Waiting on it. |
…stop test stopping_the_mac_joins_its_threads asserted that a Weak to the device could no longer be upgraded the instant the caller dropped its Arc. The receive thread holds a strong reference for the length of each poll, so under load it can still be inside one when the caller drops; the thread then releases the last reference and the device is freed a moment later. That failed about 3 runs in 100 when run alone on a loaded host, with "assertion failed: probe.upgrade().is_none()". The device behaves correctly: Drop calls stop, which skips joining the current thread. Wait (bounded) for the device to be freed and check that its running flag clears, which is what dropping a running device must achieve. A new test pins the case deterministically: a reference held on another thread keeps the device alive and running past the caller's drop, and releasing it there stops the device.
stop_joins_the_threads_and_removes_the_timers failed about once in 300 runs with siblings loaded: "start arms the deadlines the chip holds". The chip was fine. A timer callback clears its slot in the armed table on entry and arms the next deadline before it returns, so a repeating output shows no pending timer for that short span. The test read the table without regard to a callback in flight, and with a 1000-tick period a callback is often in flight under load. Read the table through a helper that holds the callback gate, which a callback holds from entry to return, so the read sees the table at rest. The assertion is unchanged. A second test reads it 20000 times against a running periodic output; without the gate it fails 29 runs in 30.
jamesbraid
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
isp1040: move the test clock under the state lock
Cause:
Rig::clockstored the fake time and notified the condition variable without taking the state lock. A move that landed after the ring thread read the clock and before it started waiting was lost, and the test hung inwait_idle(6 runs in 1000 when run alone).Fix: the clock now moves through
Core::set_test_clock, which takes the state lock first. A new test moves the clock past 2000 selection timeouts; without the lock it fails in 91 of 100 runs.Test-only: guests never use the fake clock, so their behavior is unchanged.
ioc3_eth: wait for the device to be freed in the stop test
Cause: the test asserted that a
Weakto the device could not be upgraded the instant the caller dropped itsArc. The receive thread can hold a strong reference during a poll, so the device is freed a moment later (about 3 runs in 100 under load). The device itself behaves correctly.Fix: the test waits, bounded, for the device to be freed and for its running flag to clear. A new test holds a reference on another thread to pin the case deterministically.
Test-only: the change touches only test code, so guests are unaffected.
ioc3: read the pending timer only between callbacks in the stop test
Cause: a timer callback clears its armed-table slot on entry and arms the next deadline before it returns, so a repeating output can show no pending timer for a short span. The test read the table while a callback could be in flight (about 1 run in 300 under load).
Fix: the table is read through a helper that holds the callback gate, so the read sees the table at rest. A new test reads it 20000 times against a running periodic output; without the gate it fails 29 runs in 30.
Test-only: the change touches only test code, so guests are unaffected.
Tested with plain
cargo test --release --lib, with and without jitv2.