Early stopping doesn't work if a Drop impl tries to acquire locks
- Dominant language
- Rust
- Stars
- 1.1k
- Forks
- 59
- Avg merge
- 4d 2h
- Merged PRs (30d)
- 15
Description
This test panics in the `Drop` impl of `PoolItem`:
```rust
#[test]
fn max_steps_continue_lock_during_drop() {
let mut config = Config::new();
config.max_steps = MaxSteps::ContinueAfter(10);
let scheduler = DfsScheduler::new(None, false);
let runner = Runner::new(scheduler, config);
runner.run(|| {
#[derive(Clone)]
struct Pool {
items: Arc>>,
}
struct PoolItem {
pool: Arc>>,
item: usize,
}
impl Pool {
fn new(length: usize) -> Self {
Self {
items: Arc::new(Mutex::new((0..length).collect())),
}
}
fn get(&self) -> Option {
let mut items = self.items.lock().unwrap();
let item = items.pop_front()?;
Some(PoolItem {
pool: self.items.clone(),
item,
})
}
}
impl Drop for PoolItem {
fn drop(&mut self) {
let mut items = self.pool.lock().unwrap();
items.push_back(self.item);
}
}
let pool = Pool::new(10);
let threads: Vec<_> = (0..3).map(|_| {
let pool = pool.clone();
thread::spawn(move || {
let _item = pool.get();
thread::yield_now();
})
}).collect();
for thd in threads {
thd.join().unwrap();
}
})
}
```
The problem is that `ContinueAfter` makes us stop the test early, while some `PoolItem`s are still alive. During cleanup we drop the `Generator` for each thread, which tries to safely clean up by unwinding their stacks, including dropping the `PoolItem`. But `PoolItem`'s drop tries to acquire a lock, which is no longer allowed once `ContinueAfter` has triggered.
I'm not totally sure what to do here:
* We could just leak the continuation's stack...
* We could do something smarter during teardown, allowing threads to continue running until termination or deadlock, under the assumption that the only code that can run once we drop the continuations is cleanup code
Contributor guide
Research direction
Start with the max_steps_continue_lock_during_drop test and trace Runner::run, MaxSteps::ContinueAfter, and Generator cleanup. The issue presents two possible teardown strategies but does not establish which invariant is required, so confirm that decision before changing behavior. Done should include a regression test showing cleanup of a Drop implementation that acquires a lock no longer panics.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- testing-qa
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100