oxidecomputer / oxidecomputer/propolis
server serial task's send-to-listeners future is not cancel-safe
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 270
- Forks
- 42
- Avg merge
- 4d 5h
- Merged PRs (30d)
- 6
Description
Discovered this during some ad hoc testing with multiple connections to a single guest's serial console, where I observed some data being sent a second time to a listener that had already received it.
If the serial console task has read some data from the guest that needs to be written to listeners, it generates a future to do that writing and then includes it in the main select! that determines the task's next action (lines 116-125 below where cur_output is Some): https://github.com/oxidecomputer/propolis/blob/1b34075ee5dd4235dc7eb46061e1e648fb4d121b/bin/propolis-server/src/lib/serial/mod.rs#L109-L130
cur_output is only set to None if this future is the one chosen by select!: https://github.com/oxidecomputer/propolis/blob/1b34075ee5dd4235dc7eb46061e1e648fb4d121b/bin/propolis-server/src/lib/serial/mod.rs#L226-L230
But if this branch is not chosen, for_each_concurrent may have sent the current output bytes to some listeners and not to others. (The underlying websocket send may not be cancel-safe either but I haven't looked at its implementation.) This will cause the same data to be re-sent in the next loop iteration.
An approach along the lines suggested in RFD 400 section 5.2 might help here (and other approaches may work too):
- If there's already a send-to-clients future active, try to resolve it
- Else if there's no such future but there is some data waiting to be sent, create a new future to try to send that data
- Else try to read more data from the guest
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in bin/propolis-server/src/lib/serial/mod.rs around lines 109-130 and 226-230, then read RFD 400 section 5.2. Trace how select! and for_each_concurrent handle cur_output cancellation. Done means an interrupted send cannot cause already-delivered serial data to be sent again to listeners.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- operating-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100