Azure / Azure/azure-sdk-for-rust
[Event Hubs] EventProcessor::run() overwrites a shutdown requested before it started
- Dominant language
- Rust
- Stars
- 884
- Forks
- 365
- Avg merge
- 2d 19h
- Merged PRs (30d)
- 109
Description
## Summary
`EventProcessor::run()` sets `is_running` to `true` unconditionally when it starts. A caller that spawns `run()` and then calls `shutdown()` before the spawned task reaches that line loses the shutdown, and the processor runs forever.
## Motivation
`shutdown()` sets `is_running` to `false`, and `run()` overwrites it with `true` at the top of its body. Spawning `run()` and stopping it shortly afterward is the ordinary shape for a short lived processor and for a test, so the window is easy to hit and the result is a processor that no later `shutdown()` call can stop, because the flag is already what `run()` expects. The failure looks like a hang rather than an error, so it is attributed to the broker or to the load balancer instead of to the flag.
## Proposal
- Do not let `run()` clear a shutdown that was requested before it started. Read the flag rather than overwriting it, or track the run generation so a stale shutdown cannot be lost.
- Decide and document what `run()` does when it starts after a `shutdown()`. Returning `Ok(())` at once is reasonable, and so is treating each `run()` call as a fresh generation, but the two differ for a caller.
- Add a test that calls `shutdown()` before `run()` observes the flag, and that asserts `run()` returns rather than looping.
### Note
Found while working on #5096, which moves the consumers map onto the processor so that `shutdown()` can reach the receivers. This flag race is a separate defect and is deliberately out of scope there.
Contributor guide
Assessment
This issue has not been assessed yet.