Skip to content

Jline command line editor#18

Merged
fglock merged 6 commits intomasterfrom
jline-command-line-editor
Jul 9, 2025
Merged

Jline command line editor#18
fglock merged 6 commits intomasterfrom
jline-command-line-editor

Conversation

@fglock
Copy link
Copy Markdown
Owner

@fglock fglock commented Jul 9, 2025

No description provided.

@fglock fglock merged commit 6d507d2 into master Jul 9, 2025
2 checks passed
@fglock fglock deleted the jline-command-line-editor branch July 9, 2025 19:52
fglock added a commit that referenced this pull request Apr 24, 2026
… build

Restores TT chomp.t / directive.t / text.t "Can't call method clone on
undef" that the earlier commit "drop asymmetric incref in
RuntimeScalar.addToArray" exposed — without reintroducing the DBIC
TxnScopeGuard #18 refcount leak.

## Root cause

During `my $arr = [ Foo->new(1), Foo->new(2) ]`:
  1. Foo->new(1) returns a RuntimeScalar referencing a blessed hash.
     It's added to the literal's RuntimeArray.
  2. Foo->new(2) constructs its value.  Somewhere inside, `setLarge*`
     triggers MortalList.flush() to process pending deferred decrefs.
  3. If the Foo(1) element is marked as mortal pending and hasn't
     yet been tied down by createReferenceWithTrackedElements (which
     runs only at literal-end), the mid-build flush decrements its
     refCount to 0 and fires DESTROY on Foo(1).
  4. The anon-array-literal ends up with a DESTROYed Foo(1) — later
     accesses fail with "Can't call method X on undefined".

## Previous (incorrect) approach

c8f669b solved this by adding a refcount-incref to every
RuntimeScalar.addToArray call.  The incref PINS each element so the
mid-build flush can't drop refCount to 0.  Unfortunately the incref
also applies to paths that are NOT anon-array-literal construction —
e.g. RuntimeList iteration used by some arg-flattening paths — and
no matching decref exists, leaking +1 refCount per blessed value.
That phantom refCount broke DBIC's zombie-ref double-DESTROY
detection (t/storage/txn_scope_guard.t test 18).

## New approach: narrow envelope at the emit site

Wrap the *entire* anon-array-literal construction in
MortalList.suppressFlush(true) ... suppressFlush(prev); if (prev) flush().
Keeps the window of "mid-build flush cannot fire" as short as
possible, and affects *only* the anon-array-literal call site
(EmitLiteral), not every addToArray caller.  Refcount accounting in
every other path stays symmetric — DBIC's zombie-refs test keeps
passing.

Bytecode emitted (prolog):
    ICONST_1
    INVOKESTATIC MortalList.suppressFlush(Z)Z
    ISTORE wasFlushingSlot
    <... existing element-add loop + createReferenceWithTrackedElements ...>

Bytecode emitted (epilog, keeping the reference on TOS):
    ILOAD wasFlushingSlot
    INVOKESTATIC MortalList.suppressFlush(Z)Z
    POP
    ILOAD wasFlushingSlot
    IFNE skipFlush
    INVOKESTATIC MortalList.flush()V
  skipFlush:

The IFNE ensures we don't flush when flushing was previously
suppressed by an OUTER context — in that case the outer unwinder
will handle it.

## Test plan

- make dev — PASS
- DBIC 15-test indicator set — **15/15 PASS (0 real failures)**
- Template t/chomp.t — 78/78 PASS (was "Can't call method clone on
  undef" after the revert)
- DBIC t/storage/txn_scope_guard.t — 18/18 PASS

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
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.

1 participant