fix: Detach threads in ChildFunc to prevent memory leaks#22
Merged
Conversation
toddr-bot
force-pushed
the
koan.toddr.bot/fix-issue-12
branch
from
803c6d1 to
3f1b9b0
Compare
toddr-bot
added a commit
that referenced
this pull request
Mar 21, 2026
Author
Rebase: fix: Detach threads in ChildFunc to prevent memory leaksBranch Diff: 3 files changed, 70 insertions(+), 1 deletion(-) Actions
CICI failed initially, fixed on attempt 1. Automated by Kōan |
toddr
approved these changes
Mar 22, 2026
Member
|
@toddr-bot if windows doesn't work in this scenario then do we need to be documenting and maybe even open an issue that this is unsupported to explore if there's a way to do it? |
When ChildFunc creates a thread via threads->new() in ithreads mode, the thread object was never joined or detached. This caused leftover thread mappings to accumulate over time, growing the VM footprint. Since the return value of the thread is not used, detach() is the correct cleanup method. Detached threads release their resources automatically when they finish executing. Fixes #12 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Here's a summary of what was changed: - **Created GitHub issue #30** (`t/ithread.t is failing on Windows due to ithreads socket handling`) to document the unsupported Windows scenario and track exploration of potential fixes, per @toddr's review request - **Updated skip message in `t/ithread.t`** to reference the new issue (#30), so anyone encountering the skip can find context and track progress
toddr-bot
force-pushed
the
koan.toddr.bot/fix-issue-12
branch
from
0dc9754 to
9b7b4e3
Compare
Author
Rebase: fix: Detach threads in ChildFunc to prevent memory leaksBranch Diff: 4 files changed, 76 insertions(+), 1 deletion(-) Review feedback was analyzed and applied. ChangesHere's a summary of what was changed:
Actions
CICI passed. Automated by Kōan |
toddr-bot
added a commit
that referenced
this pull request
Mar 22, 2026
In ithreads mode, the parent's copy of the client socket was not closed after spawning the handler thread. In Perl ithreads, threads->new() clones the interpreter and dup()s file descriptors, so both parent and child thread have independent handles. Not closing the parent's copy caused socket handle conflicts on Windows, leading to premature process termination after the second client connected. This mirrors the existing behavior in fork mode, where the parent already closes its copy via ServClose(). Combined with the thread detachment fix from PR #22 (cbf889d), this resolves the root causes of the t/ithreadm.t Windows failure, so the Windows skip is removed. Fixes #19 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
toddr-bot
added a commit
that referenced
this pull request
Mar 22, 2026
In ithreads mode, the parent's copy of the client socket was not closed after spawning the handler thread. In Perl ithreads, threads->new() clones the interpreter and dup()s file descriptors, so both parent and child thread have independent handles. Not closing the parent's copy caused socket handle conflicts on Windows, leading to premature process termination after the second client connected. This mirrors the existing behavior in fork mode, where the parent already closes its copy via ServClose(). Combined with the thread detachment fix from PR #22 (cbf889d), this resolves the root causes of the t/ithreadm.t Windows failure, so the Windows skip is removed. Fixes #19 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When
ChildFunccreates a thread viathreads->new()in ithreads mode, the thread object was never joined or detached. This caused leftover thread mappings to accumulate over time, growing the VM footprint. Since the return value is not used, calling->detach()is the correct fix — detached threads release their resources automatically when they finish.Fixes #12
Changes
->detach()on threads created inChildFunc(2-line change inlib/Net/Daemon.pm)t/ithread-detach.ttest verifying no joinable threads remain afterChildFuncTest plan
prove -Ilib t/— 45 tests, 11 files)t/ithread-detach.tverifies thread detachment (skips on systems without ithreads, consistent with existing ithread tests)Generated by Kōan /fix
Quality Report
Changes: 3 files changed, 70 insertions(+), 1 deletion(-)
Code scan: clean
Tests: passed (OK)
Branch hygiene: clean
Generated by Kōan post-mission quality pipeline