Skip to content

Commit 003b368

Browse files
docs: update design doc with DBI Active flag fix results (step 5.56)
Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1 parent 7035549 commit 003b368

3 files changed

Lines changed: 69 additions & 19 deletions

File tree

dev/modules/dbix_class.md

Lines changed: 67 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -193,11 +193,10 @@ a module whose `.pod`/`.pm` files were previously installed as read-only (0444),
193193
| 5.36 | Fix `%{{ expr }}` parser disambiguation inside dereference context | `Parser.java`, `StatementResolver.java`, `Variable.java` | DONE |
194194
| 5.37 | Fix `//=`, `||=`, `&&=` short-circuit in bytecode interpreter | `BytecodeCompiler.java` | DONE |
195195

196-
**t/60core.t results** (170 tests emitted):
197-
- **ok 1–37, 39–81, 127–170**: All real tests pass
198-
- **not ok 38**: `-and` array condition in `find()` — returns a row instead of undef (real bug)
199-
- **not ok 82–126**: "Unreachable cached statement still active" — DESTROY-related (statement handles never `finish()`ed)
200-
- **not ok 171–175**: Garbage collection tests — expected failures (JVM has no reference counting / `weaken`)
196+
**t/60core.t results** (142 tests emitted, updated after step 5.56):
197+
- **125 ok**: All real tests pass
198+
- **not ok 82–93**: 12 "Unreachable cached statement still active" — cursors not fully consumed, need DESTROY to call finish()
199+
- **not ok 138–142**: 5 garbage collection tests — expected (JVM has no reference counting / `weaken`)
201200

202201
**Full test suite results** (314 test files, updated 2026-04-02):
203202

@@ -398,7 +397,7 @@ Of the 40 test files with real TAP failures, detailed analysis shows:
398397

399398
| Test | Failures | Root cause | Status |
400399
|------|----------|------------|--------|
401-
| `t/60core.t` tests 82-126 | 45 | "Unreachable cached statement" — DESTROY-related | Systemic |
400+
| `t/60core.t` tests 82-93 | 12 | "Unreachable cached statement" — DESTROY-related (reduced from 45 by step 5.56) | Systemic |
402401
| `t/85utf8.t` | 14 | `utf8::is_utf8` flag — JVM strings are natively Unicode | Systemic |
403402
| `t/100populate.t` | 12 | Tests 37-42/53 DESTROY-related; test 59 JDBC batch execution | Partially systemic |
404403
| `t/88result_set_column.t` | 1 | DBIx::Class's own TODO test | Not a PerlOnJava bug |
@@ -613,13 +612,47 @@ Expressions like `($x) ? @$a = () : $b = []` triggered "Modification of a read-o
613612

614613
#### Systemic — Not planned for short-term
615614

616-
- DESTROY / TxnScopeGuard (6 txn_scope_guard + 45 cached stmt + 12 populate = ~63 tests)
617615
- GC / weaken / isweak (~44 files with GC-only noise)
618-
- UTF8 flag semantics (8 tests in t/85utf8.t)
616+
- UTF8 flag semantics (8 tests in t/85utf8.t — JVM strings are natively Unicode)
617+
618+
#### Phase 6 — DBI Statement Handle Lifecycle ✅ COMPLETED
619+
620+
**Root cause**: Three compounding bugs in PerlOnJava DBI's `Active` flag management:
621+
1. `prepare()` copies ALL dbh attributes to sth including `Active=true` (DBI.java line 193)
622+
2. `execute()` never sets `Active` based on whether there are results
623+
3. Fetch methods never clear `Active` when result set is exhausted
624+
625+
In real Perl DBI: sth starts with Active=false, becomes true on execute with results,
626+
becomes false when all rows are fetched or finish() is called.
627+
628+
| Step | What | Impact | Status |
629+
|------|------|--------|--------|
630+
| 5.56 | Fix sth Active flag lifecycle: false after prepare, true after execute with results, false on fetch exhaustion. Use mutable RuntimeScalar (not read-only scalarFalse). Close previous JDBC ResultSet on re-execute. | t/60core.t: 45→12 cached stmt failures | ✅ Done |
631+
632+
#### Phase 7 — Transaction Scope Guard Cleanup (targets 12 t/100populate.t tests)
633+
634+
**Root cause**: `TxnScopeGuard::DESTROY` never fires → no ROLLBACK on exception →
635+
`transaction_depth` stays elevated permanently.
636+
637+
**Approach**: Cannot fix via general DESTROY (bless happens in constructor, wrong DVM scope).
638+
Best option is patching `_insert_bulk` and other callers to use explicit try/catch rollback
639+
instead of relying on DESTROY.
640+
641+
| Step | What | Impact | Status |
642+
|------|------|--------|--------|
643+
| 5.58 | Patch `_insert_bulk` with explicit try/catch rollback | 12 (t/100populate.t) | |
644+
| 5.59 | Audit other txn_scope_guard callers for similar issues | Future test coverage | |
645+
646+
#### Phase 8 — Remaining Dependency Fixes
647+
648+
| Step | What | Impact | Status |
649+
|------|------|--------|--------|
650+
| 5.60 | Sub-Quote hints.t tests 4-5 (${^WARNING_BITS} round-trip) | 2 (Sub-Quote) | |
651+
| 5.61 | `overload::constant` support | 2 (Sub-Quote hints.t 9,14) | |
619652

620653
### Progress Tracking
621654

622-
#### Current Status: Steps 5.52-5.55 complete
655+
#### Current Status: Step 5.56 complete (DBI Active flag lifecycle)
623656

624657
#### Key Test Results (2026-04-02)
625658

@@ -632,7 +665,7 @@ Expressions like `($x) ? @$a = () : $b = []` triggered "Modification of a read-o
632665
| t/storage/base.t | **0** | Was 1 |
633666
| t/search/related_strip_prefetch.t | **0** | |
634667
| t/relationship/custom_opaque.t | **0** | Was 2, fixed by autovivification bug fix |
635-
| t/60core.t | 45 (DESTROY) | All are "cached stmt still active" — DESTROY not implemented |
668+
| t/60core.t | 17 (12 cached + 5 GC) | Reduced from 50 by step 5.56 (Active flag lifecycle fix). Remaining 12 need DESTROY. |
636669

637670
#### Completed Work
638671

@@ -773,7 +806,23 @@ Expressions like `($x) ? @$a = () : $b = []` triggered "Modification of a read-o
773806
(was 152KB without hooks, causing "Can't bless non-reference value" on thaw).
774807
- Files changed: `Storable.java`
775808

776-
### DBIx::Class Full Test Suite Results (updated 2026-04-01)
809+
**Step 5.56 (2026-04-02):**
810+
- Fixed DBI sth Active flag lifecycle to match real DBI behavior
811+
- `prepare()` now sets sth Active=false (was inheriting dbh's Active=true via setFromList)
812+
- `execute()` sets Active=true only for SELECTs with result sets, false for DML
813+
- `fetchrow_arrayref()` and `fetchrow_hashref()` set Active=false when no more rows
814+
- `execute()` now closes previous JDBC ResultSet before re-executing (resource leak fix)
815+
- Used mutable `new RuntimeScalar(false)` instead of read-only `scalarFalse` constant,
816+
fixing "Modification of a read-only value attempted" in DBI.pm `finish()`
817+
- Impact: t/60core.t goes from 50 failures (45 cached stmt + 5 GC) to 17 (12 cached + 5 GC)
818+
The 33 fixed failures were: stale Active=true from prepare, DML leaving Active=true,
819+
and exhausted cursors still showing Active=true
820+
- Remaining 12 are SELECTs where cursor was opened but not fully consumed, needing DESTROY
821+
to call finish() on scope exit
822+
- Files changed: `DBI.java`
823+
- Commit: `3de38f462`
824+
825+
### DBIx::Class Full Test Suite Results (updated 2026-04-02)
777826

778827
**92 test programs (66 active, 26 skipped)**
779828

@@ -789,10 +838,9 @@ Expressions like `($x) ? @$a = () : $b = []` triggered "Modification of a read-o
789838

790839
| Test | Total Failed | GC Failures | Real Failures | Root Cause |
791840
|------|-------------|-------------|---------------|------------|
792-
| t/60core.t | 50 | 5 | 45 | "Unreachable cached statement still active" — DESTROY-related |
841+
| t/60core.t | 17 | 5 | 12 | "Unreachable cached statement" — 12 remaining after Active flag fix (step 5.56), need DESTROY |
793842
| t/100populate.t | 17 | 5 | 12 | Transaction depth (DESTROY), JDBC batch execution |
794843
| t/85utf8.t | 13 | 5 | 8 | UTF-8 byte handling (JVM strings natively Unicode) |
795-
| t/60core.t test 38 | 1 | 0 | 1 | `-and` array condition in `find()` |
796844

797845
**Previously miscounted as having real failures (actually all GC-only):**
798846

@@ -819,10 +867,12 @@ Expressions like `($x) ? @$a = () : $b = []` triggered "Modification of a read-o
819867
| leaks.t | 5/9 | 4 failures all weaken-related |
820868

821869
### Next Steps
822-
1. Remaining real failures are systemic: DESTROY/TxnScopeGuard (57 tests), UTF-8 flag (8 tests), -and condition (1 test)
823-
2. Investigate remaining Sub-Quote failures: test 24 (syntax error line numbering), test 27 (weaken/GC)
824-
3. Long-term: Investigate ASM Frame.merge() crash (root cause behind InterpreterFallbackException fallback)
825-
4. Pragmatic: Accept GC-only failures as known JVM limitation; consider `DBIC_SKIP_LEAK_TESTS` env var
870+
1. Remaining real failures are systemic: DESTROY/TxnScopeGuard (12 t/60core.t + 12 t/100populate.t), UTF-8 flag (8 tests)
871+
2. Phase 7: TxnScopeGuard fix for t/100populate.t (explicit try/catch rollback)
872+
3. Phase 8: Remaining dependency module fixes (Sub-Quote hints)
873+
4. Investigate remaining Sub-Quote failures: test 24 (syntax error line numbering), test 27 (weaken/GC)
874+
5. Long-term: Investigate ASM Frame.merge() crash (root cause behind InterpreterFallbackException fallback)
875+
6. Pragmatic: Accept GC-only failures as known JVM limitation; consider `DBIC_SKIP_LEAK_TESTS` env var
826876

827877
### Open Questions
828878
- `weaken`/`isweak` absence causes GC test noise but no functional impact — Option B (accept) or Option C (skip env var)?

src/main/java/org/perlonjava/backend/bytecode/Opcodes.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2145,7 +2145,7 @@ public class Opcodes {
21452145
* Used for (list)[indices] syntax in the interpreter.
21462146
* Format: LIST_SLICE rd list_reg indices_reg
21472147
*/
2148-
public static final short LIST_SLICE = 452;
2148+
public static final short LIST_SLICE = 453;
21492149

21502150
private Opcodes() {
21512151
} // Utility class - no instantiation

src/main/java/org/perlonjava/core/Configuration.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public final class Configuration {
3333
* Automatically populated by Gradle/Maven during build.
3434
* DO NOT EDIT MANUALLY - this value is replaced at build time.
3535
*/
36-
public static final String gitCommitId = "213ed5e6f";
36+
public static final String gitCommitId = "a36479061";
3737

3838
/**
3939
* Git commit date of the build (ISO format: YYYY-MM-DD).

0 commit comments

Comments
 (0)