Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli: tunnels can sporadically become unresponsive #181286

Merged
merged 1 commit into from
May 2, 2023

Conversation

connor4312
Copy link
Member

Last iteration I moved some RPC logic to use Tokios "codecs" to give
them cancellation safety. These operate on streams of input data.

While this worked at first, I failed to take into account that the byte
buffer we read from the stream could have more data than just the
current message under load scenarios. We were discarding any extra data
from the subsequent message. In most cases caused the next message
"length" to be read from the middle of the next message, which usually
(when parsed as a u32) was some number of gigabytes, then causing the
connection to stall forever.

Fixes #181284

Last iteration I moved some RPC logic to use Tokios "codecs" to give
them cancellation safety. These operate on streams of input data.

While this worked at first, I failed to take into account that the byte
buffer we read from the stream could have _more_ data than just the
current message under load scenarios. We were discarding any extra data
from the subsequent message. In most cases caused the next message
"length" to be read from the middle of the next message, which usually
(when parsed as a u32) was some number of gigabytes, then causing the
connection to stall forever.

Fixes #181284
@connor4312 connor4312 self-assigned this May 1, 2023
@connor4312 connor4312 added the candidate Issue identified as probable candidate for fixing in the next release label May 1, 2023
@connor4312 connor4312 added this to the April 2023 milestone May 1, 2023
@sandy081 sandy081 requested a review from aeschli May 2, 2023 08:39
@aeschli
Copy link
Contributor

aeschli commented May 2, 2023

Testing the fix by running from sources

  • starting the tunnel from the Code OSS, Account menu
  • opening the tunnel in vscode.dev with the link given by the notification

Without the fix, vscode.dev is not not showing any files in the file explorer, it's idling and can't connect to the tunnel. With the fix, loading works, but

  • initial loading takes a long time. It takes almost a minute until the server reports that a client connection has been opened. Once that happens, all loads fine. Files can be accesssed
  • however, when reloading, in no longer can show the folder. Server log shows error handshaking session for all client connections
2023-05-02 12:22:13.387 [info] [2023-05-02 12:22:13] info [tunnels::connections::relay_tunnel_host] Opened new client on channel 51

2023-05-02 12:22:13.408 [info] [2023-05-02 12:22:13] error [tunnels::connections::relay_tunnel_host] error handshaking session: EOF

@aeschli
Copy link
Contributor

aeschli commented May 2, 2023

Using bisect on insiders builds, it looks like the reload bug is present even in 1.77 insiders.
Created #181319

@aeschli aeschli merged commit 657bb89 into release/1.78 May 2, 2023
@aeschli aeschli deleted the connor4312/issue181284-candidate branch May 2, 2023 10:59
@github-actions github-actions bot locked and limited conversation to collaborators Jun 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
candidate Issue identified as probable candidate for fixing in the next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants