|
4 | 4 |
|
5 | 5 | **Module**: DBIx::Class 0.082844 |
6 | 6 | **Test command**: `./jcpan -t DBIx::Class` |
7 | | -**Branch**: `feature/dbix-class-fixes` |
8 | | -**PR**: https://github.com/fglock/PerlOnJava/pull/415 (original), PR TBD (current) |
9 | | -**Status**: Phase 5 — Fix runtime issues iteratively |
| 7 | +**Branch**: `feature/dbix-class-destroy-weaken` |
| 8 | +**PR**: https://github.com/fglock/PerlOnJava/pull/415 (original, Phases 1–6), current PR TBD |
| 9 | +**Status**: Phase 9 — Re-baseline after DESTROY/weaken merge (PR #464) |
10 | 10 |
|
11 | 11 | ## Dependency Tree |
12 | 12 |
|
@@ -903,21 +903,148 @@ instead of relying on DESTROY. |
903 | 903 | | leaks.t | 5/9 | 4 failures all weaken-related | |
904 | 904 |
|
905 | 905 | ### Next Steps |
906 | | -1. Remaining real failures are systemic: DESTROY/TxnScopeGuard (12 t/60core.t + 12 t/100populate.t), UTF-8 flag (8 tests) |
907 | | -2. Phase 7: TxnScopeGuard fix for t/100populate.t (explicit try/catch rollback) |
908 | | -3. Phase 8: Remaining dependency module fixes (Sub-Quote hints) |
909 | | -4. Investigate remaining Sub-Quote failures: test 24 (syntax error line numbering), test 27 (weaken/GC) |
| 906 | +1. **P0: Fix premature DESTROY** during `DBICTest::populate_schema()` — 20 tests blocked |
| 907 | +2. **P1: Fix refcount cascade** at scope exit (objects stay at refcnt 1, should reach 0) |
| 908 | +3. **P2: Triage remaining real failures** (cascading delete, new DESTROY interactions) |
| 909 | +4. Phase 8: Remaining dependency module fixes (Sub-Quote hints) |
910 | 910 | 5. Long-term: Investigate ASM Frame.merge() crash (root cause behind InterpreterFallbackException fallback) |
911 | | -6. Pragmatic: Accept GC-only failures as known JVM limitation; consider `DBIC_SKIP_LEAK_TESTS` env var |
| 911 | +6. UTF-8 flag semantics: 8 tests (systemic JVM limitation, unchanged by DESTROY/weaken) |
912 | 912 |
|
913 | 913 | ### Open Questions |
914 | | -- `weaken`/`isweak` absence causes GC test noise but no functional impact — Option B (accept) or Option C (skip env var)? |
| 914 | +- Premature DESTROY: is the refcount being decremented too aggressively in `populate()`/`dbh_do()`/`BlockRunner` call chain? |
| 915 | +- GC leak at END: is the `$schema` variable's `scopeExitCleanup` not decrementing properly, or is the cascade from schema → storage → dbh not propagating? |
915 | 916 | - RowParser crash: is it safe to ignore since all real tests pass before it fires? |
916 | 917 |
|
| 918 | +--- |
| 919 | + |
| 920 | +## Phase 9: Re-baseline After DESTROY/weaken (2026-04-10) |
| 921 | + |
| 922 | +### Background |
| 923 | + |
| 924 | +PR #464 merged DESTROY and weaken/isweak/unweaken into master with refCount tracking. |
| 925 | +This fundamentally changes the DBIx::Class compatibility landscape: |
| 926 | +- `Scalar::Util::isweak()` now returns true for weakened refs |
| 927 | +- DESTROY fires for blessed objects when refCount reaches 0 |
| 928 | +- `Scope::Guard`, `TxnScopeGuard` destructors now fire |
| 929 | +- `Devel::GlobalDestruction::in_global_destruction()` works |
| 930 | + |
| 931 | +### Step 9.1: Fix interpreter fallback regressions (DONE) |
| 932 | + |
| 933 | +Two regressions from PR #464 fixed in commit `756f9a46a`: |
| 934 | + |
| 935 | +| Issue | Root cause | Fix | |
| 936 | +|-------|-----------|-----| |
| 937 | +| `ClassCastException` in `SCOPE_EXIT_CLEANUP_ARRAY` | Interpreter registers can hold unexpected types in fallback path | Added `instanceof` guards in `BytecodeInterpreter.java` | |
| 938 | +| `ConcurrentModificationException` in `Makefile.PL` | DESTROY callbacks modify global variable maps during `GlobalDestruction` iteration | Snapshot with `toArray()` in `GlobalDestruction.java` | |
| 939 | + |
| 940 | +### Step 9.2: Current test results (92 files, 2026-04-10) |
| 941 | + |
| 942 | +| Category | Count | Details | |
| 943 | +|----------|-------|---------| |
| 944 | +| Fully passing | 15 | All subtests pass including GC epilogue | |
| 945 | +| GC-only failures | ~13 | Real tests pass; END-block GC assertions fail (refcnt 1) | |
| 946 | +| Blocked by premature DESTROY | 20 | Schema destroyed during `populate_schema()` — no real tests run | |
| 947 | +| Real + GC failures | ~12 | Mix of logic bugs + GC assertion failures | |
| 948 | +| Skipped | ~26 | No DB driver / fork / threads | |
| 949 | +| Errors | ~6 | Parse errors, missing modules | |
| 950 | + |
| 951 | +**Individual test counts**: 645 ok / 183 not ok (total 828 tests emitted) |
| 952 | + |
| 953 | +### Key improvements from DESTROY/weaken |
| 954 | + |
| 955 | +| Before (Phase 5 final) | After (Phase 9) | |
| 956 | +|------------------------|-----------------| |
| 957 | +| t/60core.t: 12 "cached statement" failures | **1 failure** — sth DESTROY now calls `finish()` | |
| 958 | +| `isweak()` always returned false | `isweak()` returns true — Moo accessor validation works | |
| 959 | +| TxnScopeGuard::DESTROY never fired | DESTROY fires on scope exit | |
| 960 | +| weaken() was a no-op | weaken() properly decrements refCount | |
| 961 | + |
| 962 | +### Blocker: Premature DESTROY (20 tests) |
| 963 | + |
| 964 | +**Symptom**: `DBICTest::populate_schema()` crashes with: |
| 965 | +``` |
| 966 | +Unable to perform storage-dependent operations with a detached result source |
| 967 | + (source 'Genre' is not associated with a schema) |
| 968 | +``` |
| 969 | + |
| 970 | +**Affected tests** (all show ok=0, fail=2): |
| 971 | +t/64db.t, t/65multipk.t, t/69update.t, t/70auto.t, t/76joins.t, t/77join_count.t, |
| 972 | +t/78self_referencial.t, t/79aliasing.t, t/82cascade_copy.t, t/83cache.t, |
| 973 | +t/87ordered.t, t/90ensure_class_loaded.t, t/91merge_joinpref_attr.t, |
| 974 | +t/93autocast.t, t/94pk_mutation.t, t/104view.t, t/18insert_default.t, |
| 975 | +t/63register_class.t, t/discard_changes_in_DESTROY.t, t/resultset_overload.t |
| 976 | + |
| 977 | +**Root cause**: The schema object's refCount drops to 0 during the `populate()` |
| 978 | +call chain (`populate()` → `dbh_do()` → `BlockRunner` → `Try::Tiny` → storage |
| 979 | +operations). DESTROY fires mid-operation, disconnecting the database. The schema |
| 980 | +is still referenced by `$schema` in the test, so refCount should be >= 1. |
| 981 | + |
| 982 | +**Investigation needed**: |
| 983 | +- Trace where the schema's refCount goes from 1 → 0 during `populate_schema()` |
| 984 | +- Likely a code path that creates a temporary copy of the schema ref (incrementing |
| 985 | + refCount) then exits scope (decrementing back), but the decrement is applied to |
| 986 | + the wrong object or at the wrong time |
| 987 | +- The `BlockRunner` → `Try::Tiny` → `Context::Preserve` chain involves multiple |
| 988 | + scope transitions where refCount could be incorrectly managed |
| 989 | + |
| 990 | +### Blocker: GC leak at END time (refcnt 1) |
| 991 | + |
| 992 | +**Symptom**: All tests that complete their real content still show `refcnt 1` for |
| 993 | +DBI::db, Storage::DBI, and Schema objects at END time. The weak refs in the leak |
| 994 | +tracker registry remain defined instead of becoming undef. |
| 995 | + |
| 996 | +**Impact**: Tests report 2–20 GC assertion failures after passing all real tests. |
| 997 | +In the old plan (pre-DESTROY/weaken), these tests were counted as "GC-only failures" |
| 998 | +with no functional impact. With DESTROY/weaken, the GC tracker now sees real refcounts |
| 999 | +but the cascade to 0 doesn't happen. |
| 1000 | + |
| 1001 | +**Root cause**: When `$schema` goes out of scope at test end: |
| 1002 | +1. `scopeExitCleanup` should decrement schema's refCount to 0 |
| 1003 | +2. DESTROY should fire on schema, releasing storage (refCount → 0) |
| 1004 | +3. DESTROY should fire on storage, closing DBI handle (refCount → 0) |
| 1005 | + |
| 1006 | +Step 1 or the cascade at steps 2-3 is not happening correctly. |
| 1007 | + |
| 1008 | +### Tests with real (non-GC) failures |
| 1009 | + |
| 1010 | +| Test | ok | fail | Notes | |
| 1011 | +|------|-----|------|-------| |
| 1012 | +| t/60core.t | 91 | 7 | 1 cached stmt, 2 cascading delete (new), 4 GC | |
| 1013 | +| t/100populate.t | 36 | 10 | Transaction depth + JDBC batch + GC | |
| 1014 | +| t/752sqlite.t | 37 | 20 | Mostly GC (multiple schemas × GC assertions) | |
| 1015 | +| t/85utf8.t | 9 | 5 | UTF-8 flag (systemic JVM) | |
| 1016 | +| t/93single_accessor_object.t | 10 | 12 | GC heavy | |
| 1017 | +| t/84serialize.t | 115 | 5 | All GC (real tests pass) | |
| 1018 | +| t/88result_set_column.t | 46 | 6 | GC + TODO | |
| 1019 | +| t/101populate_rs.t | 17 | 4 | Needs investigation | |
| 1020 | +| t/106dbic_carp.t | 3 | 4 | Needs investigation | |
| 1021 | +| t/33exception_wrap.t | 3 | 5 | Needs investigation | |
| 1022 | +| t/34exception_action.t | 9 | 4 | Needs investigation | |
| 1023 | + |
| 1024 | +### Items obsoleted by DESTROY/weaken |
| 1025 | + |
| 1026 | +These items from the old plan are no longer needed: |
| 1027 | +- **Phase 7 (TxnScopeGuard explicit try/catch rollback)** — DESTROY handles this |
| 1028 | +- **"Systemic: DESTROY / TxnScopeGuard" section** — resolved by PR #464 |
| 1029 | +- **"Systemic: GC / weaken / isweak absence" section** — resolved by PR #464 |
| 1030 | +- **Open Question about weaken/isweak Option B vs C** — moot, they work now |
| 1031 | + |
| 1032 | +### Implementation Plan (Phase 9 continued) |
| 1033 | + |
| 1034 | +| Step | What | Impact | Status | |
| 1035 | +|------|------|--------|--------| |
| 1036 | +| 9.1 | Fix interpreter SCOPE_EXIT_CLEANUP + GlobalDestruction CME | Unblock all testing | DONE | |
| 1037 | +| 9.2 | Re-baseline test suite | Get current numbers | DONE | |
| 1038 | +| 9.3 | Fix premature DESTROY in populate_schema | Unblock 20 tests | | |
| 1039 | +| 9.4 | Fix refcount cascade at scope exit | Fix GC leak assertions | | |
| 1040 | +| 9.5 | Triage remaining real failures | Reduce fail count | | |
| 1041 | +| 9.6 | Re-run full suite after fixes | Updated numbers | | |
| 1042 | + |
917 | 1043 | ## Related Documents |
918 | 1044 |
|
919 | 1045 | - `dev/modules/moo_support.md` — Moo support (dependency of DBIx::Class) |
920 | 1046 | - `dev/modules/xs_fallback.md` — XS fallback mechanism |
921 | 1047 | - `dev/modules/makemaker_perlonjava.md` — MakeMaker for PerlOnJava |
922 | 1048 | - `dev/modules/cpan_client.md` — jcpan CPAN client |
923 | 1049 | - `docs/guides/database-access.md` — JDBC database guide (DBI, SQLite support) |
| 1050 | +- `dev/design/destroy_weaken_plan.md` — DESTROY/weaken implementation (PR #464) |
0 commit comments