myzhang1029 / penguin-rs

A fast TCP/UDP tunnel, transported over HTTP WebSocket.
Apache License 2.0
27 stars 1 forks source link

New congestion control #31

Closed myzhang1029 closed 5 months ago

myzhang1029 commented 6 months ago

Edit: This is not #30 but a different problem.

In the current implementation, if one task calls poll_write and get a Pending, then later a different task gets another Pending, then the first task will not be waken up. Judging from tokio::net::TcpStream, it seems that this is an incorrect implementation of AsyncWrite --- Despite that this does not really matter in our use case, since any MuxStream will only be poll_wrote by one task.

Examine tokio-rs/tokio src/io/poll_evented.rs#L73.

codecov[bot] commented 6 months ago

Codecov Report

Attention: Patch coverage is 96.29630% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 78.44%. Comparing base (27017d6) to head (f440a89). Report is 1 commits behind head on main.

Files Patch % Lines
src/mux/stream.rs 96.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #31 +/- ## ========================================== + Coverage 78.34% 78.44% +0.09% ========================================== Files 29 29 Lines 4489 4513 +24 ========================================== + Hits 3517 3540 +23 - Misses 972 973 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

myzhang1029 commented 6 months ago

This temporary workaround allows the task to linger even if the receiving side closes the websocket connection. Might allow DoS.

myzhang1029 commented 6 months ago

Can confirm this fix does not have the same problem as the workaround.