Skip to content

Conversation

littledivy
Copy link
Member

Fixes #25126

@littledivy littledivy force-pushed the ws_handshake_hang_fix branch from 798455c to 4d85486 Compare March 23, 2025 17:45
Comment on lines +264 to +265
if (this[_cancelHandle]) {
core.tryClose(this[_cancelHandle]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should there also be

this[_cancelHandle] = undefined;

here like the other block added below?

Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm testing this locally and whether or not I run the target/debug/test_server the test passes... that seems problematic
Running

cargo run --bin deno -- test -A --location=http://js-unit-tests/foo/bar --config=tests/config/deno.json --filter="websocket close ongoing handshake" tests/unit/websocket_test.ts

Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - made the test fail when test_server is not running

@littledivy littledivy enabled auto-merge (squash) March 28, 2025 03:09
@littledivy littledivy merged commit 66e03a3 into denoland:main Mar 28, 2025
17 checks passed
littledivy added a commit that referenced this pull request Mar 28, 2025
Fixes #25126

---------

Co-authored-by: Ryan Dahl <ry@tinyclouds.org>
ry added a commit that referenced this pull request Apr 7, 2025
Fixes #25126

---------

Co-authored-by: Ryan Dahl <ry@tinyclouds.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WebSocket leaves TCP connection open after calling close
2 participants