Azure / Azure/azure-sdk-for-rust

[Event Hubs] EventProcessor::run() overwrites a shutdown requested before it started

Open
#5,102 0 comments 1 reaction 1 assignee Claimed by @j7nw4r View on GitHub
Client Event Hubs
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.