Skip to content

Fix minor issues found during code review#41104

Open
chemwolf6922 wants to merge 10 commits into
masterfrom
user/chemwolf6922/fix-batch-of-minor-issues-3
Open

Fix minor issues found during code review#41104
chemwolf6922 wants to merge 10 commits into
masterfrom
user/chemwolf6922/fix-batch-of-minor-issues-3

Conversation

@chemwolf6922

Copy link
Copy Markdown
Contributor

Summary of the Pull Request

This PR fixes a batch of minor issues found during AI code review.

src/linux/init/DnsServer.h:48
Change the data type from uint16_t to size_t to avoid a potential counter overflow on valid data. As the counter needs to hold at most 2 + 65535.

src/linux/init/init.cpp:2575
Drain the pending pids on SIGCHLD. Multiple SIGCHLD will only trigger the signalfd once if not processed fast enough:
https://man7.org/linux/man-pages/man7/signal.7.html
image

src/linux/init/util.cpp:3497
Fix a wrong condition. The old logic will mask most of the errors as EINVAL.

src/windows/common/WslCoreMessageQueue.h:174
Avoid a tight race condition between submit and cancel on m_tpHandle.

src/windows/service/exe/LxssUserSession.cpp:897
Avoid wrong context order. The order is asserted in debug builds. The old wrong MountDisk -> CreateVm context will crash the debug service.

src/windows/service/exe/LxssUserSession.cpp:980
Put the set owner after the revert. So, a failed set owner does not skip the revert.

src/windows/service/exe/WslCoreVm.cpp:1003
Fix a wrong revert logic. Where the disk will be reverted to offline if it was online.

src/windows/service/exe/WslCoreVm.cpp:2755
Block more invalid mount names.

PR Checklist

  • Closes: Link to issue #xxx
  • Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • Tests: Added/updated if needed and all pass
  • Localization: All end user facing strings can be localized
  • Dev docs: Added/updated if needed
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Add test:
MountTests::MountTests::SpecifyInvalidMountName

Copilot AI review requested due to automatic review settings July 17, 2026 08:08
@chemwolf6922
chemwolf6922 requested a review from a team as a code owner July 17, 2026 08:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses a set of small correctness and robustness issues across both the Linux init components and the Windows VM/service code paths, plus adds a regression test for invalid wsl --mount --name values.

Changes:

  • Tighten validation and fix revert/cleanup logic around disk attach/mount and VHD move operations (Windows service).
  • Improve robustness of Linux init execution and SIGCHLD handling (drain waitpid, fix exec-result read condition, avoid offset overflow).
  • Add a Windows mount regression test for invalid mount names.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/windows/MountTests.cpp Adds a new test for invalid --mount --name inputs.
src/windows/service/exe/WslCoreVm.cpp Fixes disk online revert behavior and blocks additional invalid mount names.
src/windows/service/exe/LxssUserSession.cpp Fixes execution context ordering and ensures VHD revert is not skipped on ownership failure.
src/windows/common/WslCoreMessageQueue.h Adjusts threadpool work submission placement to reduce a submit/cancel race.
src/linux/init/util.cpp Fixes an incorrect condition when interpreting exec-status pipe reads.
src/linux/init/init.cpp Drains pending SIGCHLD children to avoid missing coalesced signals.
src/linux/init/DnsServer.h Changes request-offset type to avoid potential overflow for valid TCP DNS requests.

Comment thread test/windows/MountTests.cpp
Comment thread src/windows/common/WslCoreMessageQueue.h
Copilot AI review requested due to automatic review settings July 17, 2026 08:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Comment thread src/windows/common/WslCoreMessageQueue.h Outdated
Comment thread src/windows/common/WslCoreMessageQueue.h Outdated
Copilot AI review requested due to automatic review settings July 17, 2026 08:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants