Skip to content

Commit

Permalink
avoid KeyError in edge case of yamux handler
Browse files Browse the repository at this point in the history
There are two edge cases that currently trigger `KeyError`:

1. `await newStream.open()` succeeds, but Chronos injects an implicit
   suspend point when an `{.async.}` function that suspended returns.
   During this implicit suspend point, another task could get scheduled
   and close the stream, which removes it from `m.channels` again.
   `let channel = m.channels[header.streamId` then gives `KeyError`.
   More explanation of implicit suspend point on return after await:
   status-im/nim-chronos#273

2. During the `Flush the data` block, `await m.connection.readExactly`
   is called which may suspend. If it suspends, same as above, another
   task may get scheduled that closes the stream, also resulting in the
   `KeyError` below.

The `YamuxError` is subsequently properly handled below.
  • Loading branch information
etan-status committed Mar 2, 2024
1 parent 6c87348 commit a7fe0b4
Showing 1 changed file with 6 additions and 1 deletion.
7 changes: 6 additions & 1 deletion libp2p/muxers/yamux/yamux.nim
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,12 @@ method handle*(m: Yamux) {.async.} =
await m.connection.readExactly(addr buffer[0], int(header.length))
continue

let channel = m.channels[header.streamId]
let channel =
try:
m.channels[header.streamId]
except KeyError:
raise newException(YamuxError,
"Stream was cleaned up before handling data: " & $header.streamId)

Check warning on line 533 in libp2p/muxers/yamux/yamux.nim

View check run for this annotation

Codecov / codecov/patch

libp2p/muxers/yamux/yamux.nim#L532-L533

Added lines #L532 - L533 were not covered by tests

if header.msgType == WindowUpdate:
channel.sendWindow += int(header.length)
Expand Down

0 comments on commit a7fe0b4

Please sign in to comment.