Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion src/windows/wslc/services/ContainerService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -287,14 +287,16 @@ int ContainerService::Run(Session& session, const std::string& image, ContainerO
{
// Create the container
auto runningContainer = CreateInternal(session, image, runOptions);
runningContainer.SetDeleteOnClose(false);
auto& container = runningContainer.Get();

// Start the created container
WSLCContainerStartFlags startFlags{};
WI_SetFlagIf(startFlags, WSLCContainerStartFlagsAttach, !runOptions.Detach);
THROW_IF_FAILED(container.Start(startFlags, nullptr)); // TODO: Error message, detach keys

// Disable auto-delete only after successful start
runningContainer.SetDeleteOnClose(false);

// Handle attach if requested
if (WI_IsFlagSet(startFlags, WSLCContainerStartFlagsAttach))
{
Expand Down
31 changes: 20 additions & 11 deletions src/windows/wslcsession/WSLCContainer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1274,12 +1274,10 @@ std::unique_ptr<WSLCContainerImpl> WSLCContainerImpl::Open(
{
auto allocation = virtualMachine.TryAllocatePort(e.VmPort, e.Family, e.Protocol);

THROW_HR_IF_MSG(
THROW_HR_WITH_USER_ERROR_IF(
HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS),
!allocation,
"Port %hu is in use, cannot open container %hs",
e.VmPort,
dockerContainer.Id.c_str());
std::format(L"Port {} is already in use, cannot open container {}", e.VmPort, dockerContainer.Id),
!allocation);
Comment on lines +1277 to +1280
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

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

The new user-facing port-conflict strings are hard-coded here. Elsewhere in the codebase, errors surfaced via THROW_HR_WITH_USER_ERROR(_IF) typically come from wsl::shared::Localization::MessageWslc* so they can be localized (e.g., this file uses MessageWslcVolumeNotFound). Consider adding a localized message helper for this port-in-use case and using it here.

Copilot uses AI. Check for mistakes.

inserted.VmMapping.AssignVmPort(allocation);
}
Expand Down Expand Up @@ -1443,20 +1441,31 @@ void WSLCContainerImpl::MapPorts()
auto allocatedPort =
m_virtualMachine.TryAllocatePort(e.ContainerPort, e.VmMapping.BindAddress.si_family, e.VmMapping.Protocol);

THROW_HR_IF_MSG(
THROW_HR_WITH_USER_ERROR_IF(
HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS),
!allocatedPort,
"Port %hu is in use, cannot start container %hs",
e.ContainerPort,
m_id.c_str());
std::format(L"Port {} is already in use, cannot start container {}", e.VmMapping.HostPort(), m_id),
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

In host networking mode, this branch allocates a VM port using e.ContainerPort. If that allocation fails, the conflicting resource is the VM/container port, but the user error message currently reports e.VmMapping.HostPort() (Windows host port), which can differ from e.ContainerPort in host mode (e.g., -p 8080:80). Consider reporting the port you attempted to allocate (typically e.ContainerPort / VM port) here, and reserve HostPort() for failures coming from MapPort() (Windows bind).

Suggested change
std::format(L"Port {} is already in use, cannot start container {}", e.VmMapping.HostPort(), m_id),
std::format(L"Port {} is already in use, cannot start container {}", e.ContainerPort, m_id),

Copilot uses AI. Check for mistakes.
!allocatedPort);

e.VmMapping.AssignVmPort(allocatedPort);

allocatedPorts.emplace(e.ContainerPort, allocatedPort);
}
}

m_virtualMachine.MapPort(e.VmMapping);
try
{
m_virtualMachine.MapPort(e.VmMapping);
}
catch (const wil::ResultException& ex)
{
if (ex.GetErrorCode() == HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS) || ex.GetErrorCode() == HRESULT_FROM_WIN32(WSAEADDRINUSE))
{
THROW_HR_WITH_USER_ERROR(
HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS),
std::format(L"Port {} is already in use, cannot start container {}", e.VmMapping.HostPort(), m_id));
}
throw;
}
}
}

Expand Down
9 changes: 7 additions & 2 deletions test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -988,8 +988,6 @@ class WSLCE2EContainerCreateTests
TEST_METHOD(WSLCE2E_Container_Run_PortAlreadyInUse)
{
// Bug: https://github.com/microsoft/WSL/issues/14448
SKIP_TEST_NOT_IMPL();

WSL2_TEST_ONLY();

// Start a container with a simple server listening on a port
Expand All @@ -1002,9 +1000,16 @@ class WSLCE2EContainerCreateTests
GetPythonHttpServerScript(ContainerTestPort)));
result1.Verify({.Stderr = L"", .ExitCode = 0});

auto containersBefore = ListAllContainers().size();

// Attempt to start another container mapping the same host port
auto result2 = RunWslc(std::format(L"container run -p {}:{} {}", HostTestPort1, ContainerTestPort, DebianImage.NameAndTag()));
result2.Verify({.ExitCode = 1});
VERIFY_IS_TRUE(result2.Stderr.has_value() && result2.Stderr.value().find(L"is already in use") != std::wstring::npos);

// Verify the failed container was not left behind
auto containersAfter = ListAllContainers().size();
VERIFY_ARE_EQUAL(containersBefore, containersAfter);
}

// https://github.com/microsoft/WSL/issues/14433
Expand Down