Skip to content

Fix class initialization deadlock in AbstractArrayItem and AbstractMapItem#517

Merged
david-waltermire merged 1 commit intodevelopfrom
fix/class-initialization-deadlock
Dec 7, 2025
Merged

Fix class initialization deadlock in AbstractArrayItem and AbstractMapItem#517
david-waltermire merged 1 commit intodevelopfrom
fix/class-initialization-deadlock

Conversation

@david-waltermire
Copy link
Contributor

@david-waltermire david-waltermire commented Dec 7, 2025

Summary

  • Convert static field initializers in AbstractArrayItem and AbstractMapItem to use lazy initialization via Lazy<T>
  • This prevents class initialization deadlock when JUnit 5's SEPARATE_THREAD timeout mode causes concurrent class loading from multiple threads

Problem

The deadlock occurred because:

  1. Static ARGUMENTS field initialization called IIntegerItem.type() / IAnyAtomicItem.type()
  2. This triggered initialization of other classes
  3. When multiple threads competed for class locks during parallel test execution, they could deadlock
  4. JUnit 5's timeout thread (in SEPARATE_THREAD mode) could trigger class initialization simultaneously with the test thread

Solution

Convert the problematic static fields to use Lazy<> pattern:

  • ARGUMENTS field now uses Lazy<List<IArgument>>
  • EMPTY singleton now uses Lazy<IArrayItem<?>>/Lazy<IMapItem<?>>

This defers initialization until first use within a single thread context, preventing the deadlock.

Test plan

  • Verified ArraySizeTest, MapSizeTest, MapContainsTest pass with forkCount=4
  • Verified compilation succeeds
  • Tested with parallel test execution

Fixes #515

Summary by CodeRabbit

  • Refactor
    • Optimized static initialization patterns in array and map item handling to improve startup performance and reduce memory overhead through deferred resource allocation.

✏️ Tip: You can customize this high-level summary in your review settings.

…pItem

Convert static field initializers that trigger cross-class initialization
to use lazy initialization via Lazy<T>. This prevents class initialization
deadlock when JUnit 5's SEPARATE_THREAD timeout mode causes concurrent
class loading from multiple threads.

The deadlock occurred because static ARGUMENTS field initialization called
IIntegerItem.type()/IAnyAtomicItem.type(), which triggered initialization
of other classes. When multiple threads competed for class locks during
parallel test execution, they could deadlock.

Fixes #515
@coderabbitai
Copy link

coderabbitai bot commented Dec 7, 2025

📝 Walkthrough

Walkthrough

Lazy initialization pattern replaces eager static instantiation of argument lists and empty singletons in array and map item classes, deferring object creation until first access to mitigate class initialization deadlock scenarios.

Changes

Cohort / File(s) Summary
Lazy initialization of array and map item statics
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/function/impl/AbstractArrayItem.java, core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/function/impl/AbstractMapItem.java
Converts ARGUMENTS field from List<IArgument> to Lazy<List<IArgument>> and EMPTY singleton from direct instance to Lazy<IArrayItem<?>> / Lazy<IMapItem<?>>. Updates empty() and getArguments() methods to invoke .get() on lazy-wrapped fields, deferring initialization until first use.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Both files apply the identical lazy initialization pattern with high consistency
  • Logic straightforward: wrapping static fields and adding .get() calls
  • Changes are homogeneous across files (same refactor repeated), reducing per-file reasoning overhead
  • Attention areas:
    • Verify Lazy implementation is thread-safe to address the parallel execution deadlock issue
    • Confirm initialization occurs exactly once despite concurrent .get() calls
    • Check that return type contracts remain unchanged (methods still return List<IArgument>, not Lazy<...>)

Poem

🐰 No deadlock shall hinder our speedy run,
We lazy-load singletons, one by one,
When needed, they bloom like carrots from ground,
Now parallel tests pass without a sound! 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: converting static initializers to lazy initialization to fix class initialization deadlocks in two specific classes.
Linked Issues check ✅ Passed The PR successfully implements the recommended mitigation by converting problematic static initializers to lazy initialization in AbstractArrayItem and AbstractMapItem, directly addressing the deadlock issue.
Out of Scope Changes check ✅ Passed All changes are focused on converting static field initializers to lazy initialization in the two identified classes; no unrelated modifications are present.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/class-initialization-deadlock

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 47c155a and 6f9fd7b.

📒 Files selected for processing (2)
  • core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/function/impl/AbstractArrayItem.java (4 hunks)
  • core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/function/impl/AbstractMapItem.java (4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/function/impl/AbstractMapItem.java (2)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
  • ObjectUtils (18-135)
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/function/impl/MapItemN.java (1)
  • MapItemN (23-53)
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/function/impl/AbstractArrayItem.java (2)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
  • ObjectUtils (18-135)
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/function/impl/ArrayItemN.java (1)
  • ArrayItemN (22-53)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Code
  • GitHub Check: Website
🔇 Additional comments (9)
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/function/impl/AbstractArrayItem.java (5)

39-45: Effective fix for class initialization deadlock.

Deferring the IIntegerItem.type() call until first access via lazy initialization correctly breaks the cyclic class initialization dependency that caused deadlocks under parallel class loading.


46-51: LGTM!

Clean use of method reference for the empty singleton lazy initialization.


60-64: LGTM!

The empty() accessor correctly retrieves the lazily initialized singleton via EMPTY.get().


71-74: LGTM!

The getArguments() accessor correctly retrieves the lazily initialized argument list.


25-25: LGTM on library choice for lazy initialization.

The lazy4j library provides thread-safe lazy initialization with double-checked locking semantics (volatile fields with synchronized blocks), which is appropriate for solving the class initialization deadlock issue.

core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/function/impl/AbstractMapItem.java (4)

29-29: LGTM!

Import aligns with the lazy initialization pattern used in AbstractArrayItem.java.


46-58: Consistent lazy initialization pattern.

The implementation mirrors AbstractArrayItem.java, correctly deferring IAnyAtomicItem.type() initialization to prevent the class initialization deadlock.


67-72: LGTM!

The empty() accessor correctly retrieves the lazily initialized singleton.


79-82: LGTM!

The getArguments() accessor correctly retrieves the lazily initialized argument list.


Comment @coderabbitai help to get the list of available commands and usage tips.

@david-waltermire david-waltermire merged commit 66985e9 into develop Dec 7, 2025
3 checks passed
@david-waltermire david-waltermire deleted the fix/class-initialization-deadlock branch December 7, 2025 14:34
@david-waltermire david-waltermire added this to the v3.0.0 Milestone 1 milestone Dec 8, 2025
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.

Class initialization deadlock causes test timeouts under parallel execution

1 participant