Skip to content

Commit 87b2cd8

Browse files
author
Ben Hillis
committed
Fix port relay IOCP: inherit exit event handle, fix pipe leak, harden AcceptThread
- Add InheritHandle() for m_vmTerminatingEvent so the relay child process receives a valid exit event handle via PROC_THREAD_ATTRIBUTE_HANDLE_LIST. - Close parent pipe handles with .reset() instead of .release() so ReadFile in MapRelayPort detects relay process death via pipe EOF. - Move AcceptThread I/O drain into wil::scope_exit so pending accepts are always cancelled even if ScheduleAccept throws during initialization. - Wrap initial ScheduleAccept loop in try/catch to prevent a single port failure from killing the entire accept thread. - Track accept thread liveness with std::atomic<bool> to avoid posting stale IOCP exit signals when the thread has already exited. - Make PortRelay destructor cancel pending I/O instead of __fastfail. - Add <atomic> to precomp.h.
1 parent 6d69708 commit 87b2cd8

File tree

3 files changed

+64
-33
lines changed

3 files changed

+64
-33
lines changed

src/windows/common/precomp.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ Module Name:
7070
#include <array>
7171
#include <xstring>
7272
#include <algorithm>
73+
#include <atomic>
7374
#include <map>
7475
#include <memory>
7576
#include <vector>

src/windows/wslcsession/WSLCVirtualMachine.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -838,10 +838,13 @@ void WSLCVirtualMachine::LaunchPortRelay()
838838

839839
wsl::windows::common::SubProcess process{nullptr, cmd.c_str()};
840840
process.SetStdHandles(readPipe.get(), writePipe.get(), nullptr);
841+
process.InheritHandle(m_vmTerminatingEvent.get());
841842
process.Start();
842843

843-
readPipe.release();
844-
writePipe.release();
844+
// Close the parent's copies of the child's stdin/stdout handles so that
845+
// pipe EOF is properly detected if the relay process exits.
846+
readPipe.reset();
847+
writePipe.reset();
845848
}
846849

847850
void WSLCVirtualMachine::MapRelayPort(_In_ int Family, _In_ unsigned short WindowsPort, _In_ unsigned short LinuxPort, _In_ bool Remove)

src/windows/wslrelay/localhost.cpp

Lines changed: 58 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,11 @@ struct PortRelay
347347

348348
~PortRelay()
349349
{
350-
WI_ASSERT(!Pending);
350+
if (Pending)
351+
{
352+
LOG_HR(E_UNEXPECTED);
353+
CancelIoEx(reinterpret_cast<HANDLE>(ListenSocket.get()), &Overlapped);
354+
}
351355
}
352356

353357
void LaunchRelay(const GUID& VmId)
@@ -406,7 +410,7 @@ struct PortRelay
406410

407411
void ScheduleAccept()
408412
{
409-
WI_VERIFY(!Pending);
413+
WI_ASSERT(!Pending);
410414

411415
Overlapped = {};
412416
PendingSocket.reset(WSASocket(Family, SOCK_STREAM, IPPROTO_TCP, nullptr, 0, WSA_FLAG_OVERLAPPED));
@@ -460,12 +464,42 @@ std::shared_ptr<PortRelay> CreatePortListener(uint16_t WindowsPort, uint16_t Lin
460464

461465
void AcceptThread(std::vector<std::shared_ptr<PortRelay>>& Ports, const GUID& VmId, HANDLE Iocp)
462466
{
467+
// Ensure pending I/O is always cancelled and drained, even if we exit via exception.
468+
auto drainPendingIo = wil::scope_exit([&]() {
469+
DWORD PendingCount = 0;
470+
for (auto& Entry : Ports)
471+
{
472+
if (Entry->Pending)
473+
{
474+
CancelIoEx(reinterpret_cast<HANDLE>(Entry->ListenSocket.get()), &Entry->Overlapped);
475+
PendingCount++;
476+
}
477+
}
478+
479+
while (PendingCount > 0)
480+
{
481+
DWORD Bytes{};
482+
ULONG_PTR Key{};
483+
OVERLAPPED* CompletedOverlapped{};
484+
GetQueuedCompletionStatus(Iocp, &Bytes, &Key, &CompletedOverlapped, INFINITE);
485+
if (CompletedOverlapped != nullptr && Key != 0)
486+
{
487+
reinterpret_cast<PortRelay*>(Key)->Pending = false;
488+
PendingCount--;
489+
}
490+
}
491+
});
492+
463493
// Schedule initial accepts on all ports.
464494
for (auto& Entry : Ports)
465495
{
466496
if (!Entry->Pending)
467497
{
468-
Entry->ScheduleAccept();
498+
try
499+
{
500+
Entry->ScheduleAccept();
501+
}
502+
CATCH_LOG();
469503
}
470504
}
471505

@@ -512,37 +546,15 @@ void AcceptThread(std::vector<std::shared_ptr<PortRelay>>& Ports, const GUID& Vm
512546
}
513547
}
514548

515-
// Cancel all pending I/O and drain completions from the IOCP.
516-
DWORD PendingCount = 0;
517-
for (auto& Entry : Ports)
518-
{
519-
if (Entry->Pending)
520-
{
521-
CancelIoEx(reinterpret_cast<HANDLE>(Entry->ListenSocket.get()), &Entry->Overlapped);
522-
PendingCount++;
523-
}
524-
}
525-
526-
while (PendingCount > 0)
527-
{
528-
DWORD Bytes{};
529-
ULONG_PTR Key{};
530-
OVERLAPPED* CompletedOverlapped{};
531-
GetQueuedCompletionStatus(Iocp, &Bytes, &Key, &CompletedOverlapped, INFINITE);
532-
if (CompletedOverlapped != nullptr && Key != 0)
533-
{
534-
reinterpret_cast<PortRelay*>(Key)->Pending = false;
535-
PendingCount--;
536-
}
537-
}
538549
}
539550

540551
struct MessageReader
541552
{
542-
wil::unique_event MessageReady{wil::EventOptions::ManualReset};
543-
wil::unique_event MessageConsumed{wil::EventOptions::ManualReset};
544-
std::optional<WSLC_MAP_PORT> Message;
545-
std::thread Thread;
553+
NON_COPYABLE(MessageReader);
554+
NON_MOVABLE(MessageReader);
555+
556+
MessageReader() = default;
557+
546558

547559
void Start()
548560
{
@@ -614,6 +626,11 @@ struct MessageReader
614626
WI_ASSERT(message.Header.MessageType == LxMessageWSLCMapPort);
615627
return message;
616628
}
629+
630+
wil::unique_event MessageReady{wil::EventOptions::ManualReset};
631+
wil::unique_event MessageConsumed{wil::EventOptions::ManualReset};
632+
std::optional<WSLC_MAP_PORT> Message;
633+
std::thread Thread;
617634
};
618635

619636
void wsl::windows::wslrelay::localhost::RunWSLCPortRelay(const GUID& VmId, uint32_t RelayPort, HANDLE ExitEvent)
@@ -624,11 +641,19 @@ void wsl::windows::wslrelay::localhost::RunWSLCPortRelay(const GUID& VmId, uint3
624641
THROW_LAST_ERROR_IF(!iocp);
625642

626643
std::thread acceptThread;
644+
std::atomic<bool> acceptThreadAlive{false};
627645

628646
auto stopAcceptThread = [&]() {
629647
if (acceptThread.joinable())
630648
{
631-
PostQueuedCompletionStatus(iocp.get(), 0, 0, nullptr);
649+
// Only post the exit signal if the thread is still running.
650+
// If it already exited (e.g. due to an exception), posting would leave
651+
// a stale completion in the IOCP that kills the next accept thread.
652+
if (acceptThreadAlive.load())
653+
{
654+
PostQueuedCompletionStatus(iocp.get(), 0, 0, nullptr);
655+
}
656+
632657
acceptThread.join();
633658
acceptThread = {};
634659
}
@@ -720,7 +745,9 @@ void wsl::windows::wslrelay::localhost::RunWSLCPortRelay(const GUID& VmId, uint3
720745
relays.emplace_back(e.second);
721746
}
722747

748+
acceptThreadAlive = true;
723749
acceptThread = std::thread([&, relays = std::move(relays)]() mutable {
750+
auto markDead = wil::scope_exit([&]() { acceptThreadAlive = false; });
724751
try
725752
{
726753
AcceptThread(relays, VmId, iocp.get());

0 commit comments

Comments
 (0)