fix: repair dead $RegExpLock and modernize Bind()#33
Draft
Conversation
Two bugs made the regex thread-safety lock completely non-functional:
1. threads::shared::share($RegExpLock) was gated by $forks::threads,
which is always false when using real ithreads (the $use_ithreads
guard already excludes forks.pm). The variable was never shared.
2. The lock acquisition in Accept() and t/server checked
$self->{'mode'} eq 'threads', but the mode value has been
'ithreads' (not 'threads') since ithreads support was added.
The lock was never acquired.
Fix: share() unconditionally when ithreads are loaded, and correct
the mode string to 'ithreads' in both Accept() and t/server.
Note: on modern Perl (5.10+ required for ithreads in this code),
regex is thread-safe, so this lock is defensive rather than critical.
But it should work correctly for any subclass that relies on it.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- PID file open used two-arg form with bare OUT filehandle despite having $fh from Symbol::gensym() already available. Switch to three-arg open($fh, '>', $pidfile) which is safer against metacharacter injection in the pidfile path. - Remove unused outer my $gid and my $uid declarations that were shadowed by identically-named variables in the inner if blocks. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- The DESCRIPTION still referenced Perl 5.005/5.004 as design targets; the module requires 5.006 and ithreads need 5.10+ - The --mode documentation mentioned --mode=threads which is not a recognized mode value (would produce a fatal error) - Updated the regex lock comment from the 5.00502 era to reflect that modern Perl has thread-safe regex 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
1 participant
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.
What
Fix two long-standing bugs that made
$RegExpLockcompletely non-functional in ithreads mode, plus modernize PID file handling inBind().Why
The
$RegExpLockregex thread-safety mechanism has been dead code since the ithreads mode was introduced:threads::shared::share($RegExpLock)was gated by$forks::threads, which is always false when using real ithreads (the$use_ithreadsguard on line 43 already excludesforks.pm). The variable was never actually shared between threads.The lock acquisition in
Accept()andt/serverchecked$self->{'mode'} eq 'threads', but the mode value is'ithreads'— the lock was never acquired.On modern Perl (5.10+ required for ithreads in this code), regex is thread-safe, so the lock is defensive rather than critical. But any subclass relying on
$RegExpLockfor custom synchronization would silently get no protection.How
$forks::threadscondition fromshare()call'threads'to'ithreads'inAccept()andt/serveropen(OUT, ">$pidfile")to three-argopen($fh, '>', $pidfile)using the$fhthat was already allocated but unusedmy $gid/my $uiddeclarationsTesting
All 67 tests pass. Ithread tests skipped on this system (no ithreads available) — CI will verify the ithreads code path.
🤖 Generated with Claude Code
Quality Report
Changes: 2 files changed, 15 insertions(+), 18 deletions(-)
Code scan: clean
Tests: passed (OK)
Branch hygiene: clean
Generated by Kōan post-mission quality pipeline