Branch: refs/heads/XCOMMONS-2092 Home: https://github.com/xwiki/xwiki-commons Commit: 7cb905de4773b4d1fe6fa5e9f2b490fa2aaed8cb https://github.com/xwiki/xwiki-commons/commit/7cb905de4773b4d1fe6fa5e9f2b490... Author: Simon Urli <[email protected]> Date: 2026-09-17 (Thu, 17 Sep 2026) Changed paths: M xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/main/java/org/xwiki/job/internal/ReadWriteSemaphore.java M xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/test/java/org/xwiki/job/internal/DefaultJobExecutorTest.java A xwiki-commons-core/xwiki-commons-job/xwiki-commons-job-default/src/test/java/org/xwiki/job/internal/ReadWriteSemaphoreTest.java Log Message: ----------- XCOMMONS-2092: Redesign ReadWriteSemaphore as a monitor, and add a regression test The previous commit fixed a non-atomic counter read in ReadWriteSemaphore, but kept encoding the write/read exclusion as a number of permits to acquire or release on an internal java.util.concurrent.Semaphore computed from a counter snapshot. That whole approach turned out to be fragile beyond just the non-atomicity: a shared-pool sizing rule (readers and writers share the same pool of slots, confirmed by matchingGroupPathAreBlockedPoolMultiSizeChildrenFirst letting a writer and a reader be active at once when the pool size allows it) and FIFO fairness between waiting writers are both easy to get subtly wrong when they are expressed as permit counts instead of directly against the counters. * Replace the Semaphore-and-permit-count scheme with a plain monitor (synchronized/wait/notifyAll) that checks readCounter/writeCounter/activeWriters directly: a writer can proceed once activeWriters + readCounter is below the pool size, a reader once no writer is announced or active. This removes the whole class of "permits taken and given back don't match" bugs (the exact one the previous commit's non-atomicity fix targeted, and others found in the course of validating it) by construction: there is nothing to keep resynchronized with reality any more. * Serve waiting writers in the order they called lockWrite(), via an explicit ticket: plain wait()/notifyAll() does not by itself guarantee that a slot freed up for the earliest-still-waiting writer cannot be taken by a later one instead, which matchingGroupPathAreBlockedPoolMultiSizeChildrenFirst caught happening about half the time. * Restore (as a targeted, commented wait, not a blanket workaround) the synchronization the first commit on this issue had removed between submitting a writer and submitting a reader that must queue behind it in matchingGroupPathAreBlockedPoolMultiSizeParentFirst: nothing in ReadWriteSemaphore itself can guarantee an earlier-submitted writer's worker thread reaches its own lockWrite() call before a later-submitted reader's thread reaches lockRead(), since that is a property of when each gets dispatched by the surrounding thread pool, not of the semaphore's internal bookkeeping. * Add ReadWriteSemaphoreTest with a concurrent stress test reproducing, within a few hundred operations, the original non-atomic-counter deadlock this whole change started from. Verified with the concurrency stress test (reliably deadlocks the pre-atomicity-fix code, passes against this one), and with 5000/5000, 5000/5000 and 1500/1500 repeated runs of the three DefaultJobExecutorTest methods (matchingGroupPathAreBlocked, ...ParentFirst, ...ChildrenFirst), each in its own fresh JVM, 0 failures. Co-Authored-By: Claude Sonnet 5 <[email protected]> To unsubscribe from these emails, change your notification settings at https://github.com/xwiki/xwiki-commons/settings/notifications