Fix port conflict error message and cleanup on failure (#40102)

Author: beena352

localization/strings/en-US/Resources.resw

@@ -2135,6 +2135,10 @@ For privacy information about this product please visit https://aka.ms/privacy.<
     <value>Failed to create volume '{}': {}</value>
     <comment>{FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated</comment>
   </data>
+  <data name = "MessageWslcPortInUse" xml:space = "preserve" >
+    <value>Port {} is already in use, cannot start container {}</value>
+    <comment>{FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated</comment>
+  </data>
   <data name = "MessageWslcBothDockerAndContainerFileFound" xml:space = "preserve" >
     <value>Both Dockerfile and Containerfile found. Use -f to select the file to use</value>
     <comment>{FixedPlaceholder="Dockerfile"}{FixedPlaceholder="Containerfile"}{FixedPlaceholder="-f"}Command line arguments, file names and string inserts should not be translated</comment>

src/windows/wslc/services/ContainerService.cpp

@@ -291,14 +291,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))
     {

src/windows/wslcsession/WSLCContainer.cpp

@@ -231,6 +231,16 @@ std::string ExtractContainerName(const std::vector<std::string>& names, const st
     return CleanContainerName(names[0]);
 }
 
+std::string FormatPortEndpoint(const ContainerPortMapping& portMapping)
+{
+    auto addr = portMapping.VmMapping.BindingAddressString();
+    return std::format(
+        "{}:{}/{}",
+        portMapping.VmMapping.IsIPv6() ? std::format("[{}]", addr) : addr,
+        portMapping.VmMapping.HostPort(),
+        portMapping.ProtocolString());
+}
+
 WSLCContainerMetadataV1 ParseContainerMetadata(const std::string& json)
 {
     auto wrapper = wsl::shared::FromJson<WSLCContainerMetadata>(json.c_str());
@@ -1497,20 +1507,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());
+                    wsl::shared::Localization::MessageWslcPortInUse(FormatPortEndpoint(e), m_id),
+                    !allocatedPort);
 
                 e.VmMapping.AssignVmPort(allocatedPort);
 
                 allocatedPorts.emplace(e.ContainerPort, allocatedPort);
             }
         }
 
-        m_virtualMachine.MapPort(e.VmMapping);
+        try
+        {
+            m_virtualMachine.MapPort(e.VmMapping);
+        }
+        catch (...)
+        {
+            auto result = wil::ResultFromCaughtException();
+            if (result == HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS) || result == HRESULT_FROM_WIN32(WSAEADDRINUSE))
+            {
+                THROW_HR_WITH_USER_ERROR(
+                    HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS), wsl::shared::Localization::MessageWslcPortInUse(FormatPortEndpoint(e), m_id));
+            }
+            throw;
+        }
     }
 }
 

src/windows/wslcsession/WSLCVirtualMachine.cpp

@@ -167,6 +167,11 @@ bool VMPortMapping::IsLocalhost() const
     }
 }
 
+bool VMPortMapping::IsIPv6() const
+{
+    return BindAddress.si_family == AF_INET6;
+}
+
 uint16_t VMPortMapping::HostPort() const
 {
     if (BindAddress.si_family == AF_INET6)

src/windows/wslcsession/WSLCVirtualMachine.h

@@ -86,6 +86,7 @@ struct VMPortMapping
     void Unmap();
     void Release();
     bool IsLocalhost() const;
+    bool IsIPv6() const;
     std::string BindingAddressString() const;
     void Attach(WSLCVirtualMachine& Vm);
     void Detach();

test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp

@@ -40,6 +40,7 @@ class WSLCE2EContainerRunTests
     TEST_CLASS_CLEANUP(ClassCleanup)
     {
         EnsureContainerDoesNotExist(WslcContainerName);
+        EnsureContainerDoesNotExist(WslcContainerName2);
         EnsureImageIsDeleted(DebianImage);
         EnsureImageIsDeleted(PythonImage);
 
@@ -54,6 +55,7 @@ class WSLCE2EContainerRunTests
     TEST_METHOD_SETUP(TestMethodSetup)
     {
         EnsureContainerDoesNotExist(WslcContainerName);
+        EnsureContainerDoesNotExist(WslcContainerName2);
 
         EnvTestFile1 = wsl::windows::common::filesystem::GetTempFilename();
         EnvTestFile2 = wsl::windows::common::filesystem::GetTempFilename();
@@ -384,9 +386,6 @@ class WSLCE2EContainerRunTests
 
     WSLC_TEST_METHOD(WSLCE2E_Container_Run_PortAlreadyInUse)
     {
-        // Bug: https://github.com/microsoft/WSL/issues/14448
-        SKIP_TEST_NOT_IMPL();
-
         // Start a container with a simple server listening on a port
         auto result1 = RunWslc(std::format(
             L"container run -d --name {} -p {}:{} {} {}",
@@ -397,9 +396,28 @@ class WSLCE2EContainerRunTests
             GetPythonHttpServerScript(ContainerTestPort)));
         result1.Verify({.Stderr = L"", .ExitCode = 0});
 
-        // 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});
+        // Create a second container mapping the same host port to validate the full error message
+        auto createResult =
+            RunWslc(std::format(L"container create -p {}:{} {}", HostTestPort1, ContainerTestPort, DebianImage.NameAndTag()));
+        createResult.Verify({.Stderr = L"", .ExitCode = 0});
+        auto containerId = createResult.GetStdoutOneLine();
+
+        // Attempt to start — should fail with port conflict
+        auto startResult = RunWslc(std::format(L"container start {}", containerId));
+        startResult.Verify(
+            {.Stderr = std::format(
+                 L"Port 127.0.0.1:{}/tcp is already in use, cannot start container {}\r\nError code: ERROR_ALREADY_EXISTS\r\n", HostTestPort1, containerId),
+             .ExitCode = 1});
+
+        // Clean up the created container
+        RunWslc(std::format(L"container rm {}", containerId)).Verify({.Stderr = L"", .ExitCode = 0});
+
+        // Verify 'container run' auto-cleans up on port conflict (no ghost container)
+        auto runResult = RunWslc(std::format(
+            L"container run --name {} -p {}:{} {}", WslcContainerName2, HostTestPort1, ContainerTestPort, DebianImage.NameAndTag()));
+        runResult.Verify({.ExitCode = 1});
+
+        VerifyContainerIsNotListed(WslcContainerName2);
     }
 
     // https://github.com/microsoft/WSL/issues/14433
@@ -538,6 +556,7 @@ class WSLCE2EContainerRunTests
 private:
     // Test container name
     const std::wstring WslcContainerName = L"wslc-test-container";
+    const std::wstring WslcContainerName2 = L"wslc-test-container-2";
 
     // Test environment variables
     const std::wstring HostEnvVariableName = L"WSLC_TEST_HOST_ENV";