Synchronized block locks on a mutable field

[tejasae-afk]

Spotted something in composite-builds/build-deps/jaxp/src/main/java/jaxp/sun/org/apache/xerces/internal/impl/xpath/regex/RegularExpression.java that might be worth a look: Synchronizing on a non-final field can break mutual exclusion if the field is reassigned. This patch tightens the code around that spot.
Happy to revise the approach or close this if it doesn’t fit — you know the codebase far better than I do.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 45ac8699-3447-47f7-a938-0d440bb21d21

📥 Commits

Reviewing files that changed from the base of the PR and between 4ff3344 and d096624.

📒 Files selected for processing (1)

• composite-builds/build-deps/jaxp/src/main/java/jaxp/sun/org/apache/xerces/internal/impl/xpath/regex/RegularExpression.java

---

📝 Walkthrough

Release Notes

Synchronization Fix for Mutable Field Lock Issue

• Fixed synchronization on mutable field: Resolved a threading vulnerability where code was synchronizing directly on the context field, a non-final (mutable) field. This could lead to lost mutual exclusion guarantees if the field is reassigned. The fix captures the context reference into a local variable before synchronization, ensuring consistent lock semantics.

• Applied to three matches() overloads: All three simple matches() method variants (String, char[], and CharacterIterator) now capture this.context into local variable ctx under the outer synchronized(this) lock, then synchronize on the captured reference instead of the mutable field.

• Reduced lock scope: The inner synchronization now protects a stable snapshot of the context object, improving thread safety while narrowing the critical section.

• No public API changes: Method signatures and return types remain unchanged.

⚠️ Critical Risk: Uninitialized Field Access

The implementation has a significant defect: The modified methods capture context into a local variable WITHOUT ensuring it is first initialized. Since context defaults to null and is only lazily initialized, this will cause an immediate NullPointerException when attempting to synchronize on the null reference. Other similar methods in the same class (e.g., matches(String target, int start, int end, Match match)) properly initialize context with the pattern:

if (this.context == null)
    this.context = new Context();

This initialization check is missing from the modified methods and must be added before capturing the reference.

Walkthrough

The matches(...) methods in RegularExpression.java were refactored to reduce lock scope. Each method now captures this.context into a local variable before synchronizing on that reference, narrowing the synchronization scope while maintaining the same functional behavior.

Changes

Cohort / File(s)	Summary
Synchronization Refactoring composite-builds/build-deps/jaxp/src/main/java/jaxp/sun/org/apache/xerces/internal/impl/xpath/regex/RegularExpression.java	Optimized lock scope in matches(...) methods by capturing this.context into a local variable and synchronizing on the captured reference instead of repeatedly using the shared context field.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A rabbit hops through locks so tight,
Capturing context, holding it light,
Shorter the scope, swifter the flow,
Synchronization's dance, now you will know! 🐰✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly and specifically describes the main issue addressed in the changeset: fixing synchronization on a mutable field.
Description check	✅ Passed	The description is directly related to the changeset, explaining the issue with synchronizing on a mutable field and how the patch addresses it.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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