Skip to content

fix(maintenance): fix broken double-checked locking in jobHelper()#35713

Merged
dsilvam merged 1 commit into
mainfrom
issue-589-maintenance-dcl-fix
May 15, 2026
Merged

fix(maintenance): fix broken double-checked locking in jobHelper()#35713
dsilvam merged 1 commit into
mainfrom
issue-589-maintenance-dcl-fix

Conversation

@dsilvam
Copy link
Copy Markdown
Member

@dsilvam dsilvam commented May 14, 2026

Summary

  • MaintenanceResource.jobHelper() used volatile without a synchronized block — two threads could simultaneously observe null, both call CDIUtils.getBean(), and both assign the field
  • Fixed with the full double-checked locking pattern (volatile field + synchronized inner block + double null check)
  • Extracted CDI resolution to protected resolveJobHelperBean() to enable unit testing without a live CDI container
  • Added two unit tests: concurrent initialisation (20 threads, CyclicBarrier) and single-call caching

Test plan

  • MaintenanceResourceJobHelperTest#jobHelper_shouldInitializeBeanExactlyOnce_underConcurrentAccess — 20 threads race; verifies resolveJobHelperBean() called exactly once
  • MaintenanceResourceJobHelperTest#jobHelper_shouldReturnCachedInstance_onSubsequentCalls — sequential calls return same instance without re-resolving
  • Existing MaintenanceResourceIntegrationTest should continue to pass

Fixes: dotCMS/private-issues#589

🤖 Generated with Claude Code

This PR fixes: #589

)

jobHelper() used volatile without synchronized, allowing multiple threads
to race through the null check and call CDIUtils.getBean() more than once.
Fixed with full DCL pattern; extracted resolveJobHelperBean() to enable
unit testing without CDI. Added concurrency and caching unit tests.

Refs: #589

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions github-actions Bot mentioned this pull request May 14, 2026
@github-actions github-actions Bot added the Area : Backend PR changes Java/Maven backend code label May 14, 2026
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 14, 2026

Claude finished @dsilvam's task in 2m 52s —— View job


dotCMS Backend Review

  • Load dotCMS conventions (CLAUDE.md + docs)
  • Get the Java diff
  • Spawn 4 specialized sub-agents (Security, Database, Java Standards, REST API)
  • Consolidate findings — all four returned NO_FINDINGS
  • Post consolidated review comment (here)
  • Submit formal GitHub review — skipped: I'm not authorized to approve PRs

Summary

All four specialized sub-agents returned NO_FINDINGS:

  • Security — no missing permission checks, no SQL injection, no leaked secrets, no System.getProperty/getenv. The assertBackendUser guard on calling endpoints remains intact.
  • Database / cache — no DB code in the diff.
  • Java standardsjobHelper field remains volatile, @VisibleForTesting import already present, no raw types, no Java 21 syntax in core. Chained assignment jobHelper = local = resolveJobHelperBean() is parsed right-to-left so the value is published correctly.
  • REST API — no new endpoints; the DotRuntimeException message is unchanged from the prior code, so no new information leak.

The fix correctly applies the canonical double-checked locking pattern (volatile field + synchronized inner block + double null check), and the new MaintenanceResourceJobHelperTest follows Java 11 syntax with JUnit 5 + Mockito.
· branch: issue-589-maintenance-dcl-fix

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 14, 2026

dotCMS Backend Review: No Critical, High, or Medium severity issues found. Reviewed by four specialized sub-agents (Security, Database/cache, Java standards, REST API) — all returned NO_FINDINGS. The fix correctly applies the canonical double-checked locking pattern (volatile field + synchronized inner block + double null check), and the new MaintenanceResourceJobHelperTest follows Java 11 syntax with JUnit 5 + Mockito.

@dsilvam dsilvam added this pull request to the merge queue May 15, 2026
Merged via the queue into main with commit 78adf0d May 15, 2026
52 checks passed
@dsilvam dsilvam deleted the issue-589-maintenance-dcl-fix branch May 15, 2026 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants