Skip to content

wslrelay KD: stay open after first disconnect #13441

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.

Sign up for GitHub

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

Open
asweet-confluent wants to merge 2 commits into
base: master
Choose a base branch
from
Open

wslrelay KD: stay open after first disconnect #13441

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bd7bafc
wslrelay KD: stay open after first disconnect
asweet-confluent Sep 1, 2025
5d0f62a
Apply suggestions from code review
asweet-confluent Sep 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 23 additions & 8 deletions src/windows/wslrelay/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ try
// Ensure that the other end of the pipe has connected.
wsl::windows::common::helpers::ConnectPipe(pipe.get(), (15 * 1000), {exitEvent.get()});

// Bind, listen, and accept a connection on the specified port.
// Bind and listen on the specified port (once)
const wil::unique_socket listenSocket(WSASocket(AF_INET, SOCK_STREAM, IPPROTO_TCP, nullptr, 0, WSA_FLAG_OVERLAPPED));
THROW_LAST_ERROR_IF(!listenSocket);

Expand All @@ -113,14 +113,29 @@ try

THROW_LAST_ERROR_IF(listen(listenSocket.get(), 1) == SOCKET_ERROR);

const wil::unique_socket socket(WSASocket(AF_INET, SOCK_STREAM, IPPROTO_TCP, nullptr, 0, WSA_FLAG_OVERLAPPED));
THROW_LAST_ERROR_IF(!socket);

wsl::windows::common::socket::Accept(listenSocket.get(), socket.get(), INFINITE, exitEvent.get());
for (;;)
{

// Begin the relay.
wsl::windows::common::relay::BidirectionalRelay(
reinterpret_cast<HANDLE>(socket.get()), pipe.get(), 0x1000, wsl::windows::common::relay::RelayFlags::LeftIsSocket);
const wil::unique_socket socket(WSASocket(AF_INET, SOCK_STREAM, IPPROTO_TCP, nullptr, 0, WSA_FLAG_OVERLAPPED));
THROW_LAST_ERROR_IF(!socket);

try
{
wsl::windows::common::socket::Accept(listenSocket.get(), socket.get(), INFINITE, exitEvent.get());

// Begin the relay for this connection
wsl::windows::common::relay::BidirectionalRelay(
reinterpret_cast<HANDLE>(socket.get()), pipe.get(), 0x1000,
wsl::windows::common::relay::RelayFlags::LeftIsSocket);
}
catch (...)
Copy link

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

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

The catch-all exception handler may mask specific error conditions that should cause the relay to exit (e.g., pipe closure, critical system errors). Consider catching specific exceptions or checking for terminal error conditions.

Suggested change
catch (...)
catch (const wil::ResultException& ex)

Copilot uses AI. Check for mistakes.
{
LOG_CAUGHT_EXCEPTION();

// Small delay to prevent tight loop on persistent errors
Copy link
Member

@benhillis benhillis Sep 3, 2025

Choose a reason for hiding this comment

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

What kind of persistent errors were you seeing?

Copy link
Author

@asweet-confluent asweet-confluent Sep 3, 2025

Choose a reason for hiding this comment

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

Oh, I didn't see any errors... Copilot added that and I figured it knew something I didn't. I can remove the sleep if you think it isn't needed.

Copy link
Member

@benhillis benhillis Sep 4, 2025

Choose a reason for hiding this comment

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

Yes I'd suggest removing this try / catch entirely.

Sleep(100);
Comment on lines +135 to +136
Copy link

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

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

The fixed 100ms delay may be too aggressive for rapid reconnections in kernel debugging scenarios. Consider implementing exponential backoff or making the delay configurable to balance between responsiveness and resource usage.

Copilot uses AI. Check for mistakes.
}
}

break;
}
Expand Down