Propagate LaunchInit error code - #41380
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adjusts the Linux-side mini-init launch path so that failures occurring during LaunchInit() are surfaced to the caller’s error-reporting logic (rather than being collapsed to a generic failure), improving the fidelity of error codes reported back to the Windows service during distro startup failures.
Changes:
- Removed
LaunchInit()’s internaltry/catchthat unconditionally_exit(1)’d, allowing exceptions to propagate to the caller that reports status. - Preserved the original launch socket FD for caller-side error reporting by setting
FD_CLOEXEC(instead of closing it) afterdup2(). - Added exception logging in
ProcessLaunchInitMessage()’s catch block before reporting failure.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Blue (OneBlue)
left a comment
There was a problem hiding this comment.
I'm a bit confused by this. #13028 wasn't caused by an IO error, but by a VHD that's missing critical files (like passwd) so this change wouldn't affect that behavior.
Also if we want to cover mount related errors to display the "disk corrupted error", we also need to update this condition so that the LxInitCreateInstanceStepLaunchInit failure step also emits those
|
Hi Blue (@OneBlue) . Before the user tried to fix the file system. The error was LaunchInit fails with EIO because MountInit fails with EIO. |
I was a bit hesitant to treat all EIO in this step as disk corruption. But I guess it's ok. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/linux/init/main.cpp:1502
SetCloseOnExec()is documented/implemented to return 0 on success and -1 on failure (main.cpp:3556-3559). Most call sites in this file compare the return value against< 0(e.g., main.cpp:1048, 1053). UsingTHROW_LAST_ERROR_IF(SetCloseOnExec(...))relies on implicit int-to-bool conversion and is inconsistent with the established pattern.
THROW_LAST_ERROR_IF(SetCloseOnExec(SocketFd, true));
src/windows/service/exe/WslCoreInstance.cpp:71
- Use the
EIOerrno constant instead of the magic value5for readability and to avoid baking in a numeric errno mapping.
if (((result.Result == EINVAL || result.Result == 74 || result.Result == 117) && result.FailureStep == LxInitCreateInstanceStepMountDisk) ||
(result.Result == 5 && result.FailureStep == LxInitCreateInstanceStepLaunchInit))
|
Note: used 5 instead of EIO. As the EIO in this file is the Windows EIO. Which happens to be the same as Linux. |
Summary of the Pull Request
LaunchInit currently try catch _exit(1) all exceptions. This could cause a files system failure being reported to the user as E_UNEXPECTED instead. For example: #13028 and #13237. Where the original EIO at step 3 were collapsed to E_UNEXPECTED.
This PR propagates the error out by removing LaunchInit's own try catch and rely on the caller's try catch logic. And also delays the closure of the original socket fd by setting it to CLOEXEC instead of manually close. So, the caller can still use it in catch.
This PR also adds EIO (5) at step 3 to the treated as disk corruption list.
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed
Tested by manually injecting an error in LaunchInit: