Yamux pontentially sending a WindowUpdate after it has closed the stream
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 2.6k
- Forks
- 546
- Avg merge
- 8h 18m
- Merged PRs (30d)
- 16
Description
- **Version**: js-v0_45
- **Platform**: ubuntu-22.04
- **Subsystem**: Yamux., used with ws and noise.
#### Severity: High if it is actually an issue on JS side.
#### Description:
- What you did
The ping test sometimes fails when js is the dialer and nim-libp2p the listener. https://github.com/vacp2p/nim-libp2p/actions/runs/8815571518/job/24199161935?pr=1086
- What happened
Nim is receiving a `{WindowUpdate, {}, streamId: 1, length: 127}` after it has received a `{WindowUpdate, {Fin}, streamId: 1, length: 0}`. Nim sent `{Data, {Fin}, streamId: 1, length: 0}` before receiving the messages above, so it believes the stream has been fully closed from both sides and doesn't accept the last `WindowUpdate`. It is possible that JS still hasn't received the Nim close msg. The Yamux spec says "To close a stream, either side sends a data or window update frame along with the FIN flag. This does a half-close indicating the sender will send no further data.". Does it include `WindowUpdate`?
These are the logs filtered by `streamId: 1`:
```
2024-04-24T11:58:31.0169041Z listener-1 | TRC 2024-04-24 11:58:25.717+00:00 got message topics="libp2p yamux" tid=7 h="{WindowUpdate, {Syn}, streamId: 1, length: 0}"
2024-04-24T11:58:31.0176726Z listener-1 | TRC 2024-04-24 11:58:25.717+00:00 write directly on stream topics="libp2p yamux" tid=7 h="{WindowUpdate, {Ack}, streamId: 1, length: 0}"
2024-04-24T11:58:31.0239117Z listener-1 | TRC 2024-04-24 11:58:25.726+00:00 got message topics="libp2p yamux" tid=7 h="{Data, {}, streamId: 1, length: 36}"
2024-04-24T11:58:31.0633568Z listener-1 | TRC 2024-04-24 11:58:25.771+00:00 try to send the buffer topics="libp2p yamux" tid=7 h="{Data, {}, streamId: 1, length: 20}"
2024-04-24T11:58:31.0641794Z listener-1 | TRC 2024-04-24 11:58:25.772+00:00 try to send the buffer topics="libp2p yamux" tid=7 h="{Data, {}, streamId: 1, length: 16}"
2024-04-24T11:58:31.0676154Z listener-1 | TRC 2024-04-24 11:58:25.775+00:00 try to send the buffer topics="libp2p yamux" tid=7 h="{Data, {}, streamId: 1, length: 127}"
2024-04-24T11:58:31.0683902Z listener-1 | TRC 2024-04-24 11:58:25.775+00:00 write directly on stream topics="libp2p yamux" tid=7 h="{Data, {Fin}, streamId: 1, length: 0}"
2024-04-24T11:58:31.1071982Z listener-1 | TRC 2024-04-24 11:58:25.808+00:00 got message topics="libp2p yamux" tid=7 h="{WindowUpdate, {}, streamId: 1, length: 20}"
2024-04-24T11:58:31.1112419Z listener-1 | TRC 2024-04-24 11:58:25.811+00:00 got message topics="libp2p yamux" tid=7 h="{WindowUpdate, {}, streamId: 1, length: 16}"
2024-04-24T11:58:31.1194427Z listener-1 | TRC 2024-04-24 11:58:25.818+00:00 got message topics="libp2p yamux" tid=7 h="{WindowUpdate, {Fin}, streamId: 1, length: 0}"
2024-04-24T11:58:31.1313826Z listener-1 | TRC 2024-04-24 11:58:25.831+00:00 got message topics="libp2p yamux" tid=7 h="{WindowUpdate, {}, streamId: 1, length: 127}"
```
- What you expected to happen
Not sure yet.
#### Steps to reproduce the error:
The test is flaky, but running it sometimes reproduces the situation described.
Contributor guide
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 with the Yamux subsystem and the flaky ping test described in the issue, then compare the JavaScript and Nim message logs with the quoted Yamux stream-closing rule. Reproduce the failure if possible and establish whether the final WindowUpdate is valid; done requires a decided expected behavior and corresponding regression coverage.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 28/100