From 7454314966515bb9f4d068f17c04cf0024ee1532 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Wed, 8 Jul 2026 17:41:55 +0300 Subject: [PATCH 1/2] Use notifyAll in IOTaskRunnable.kick (CodeQL java/notify-instead-of-notify-all) kick() woke a waiter with notify(). Today there is only ever a single waiter - the one background thread per IOTaskRunnable that blocks in run()'s wait() - so notify() and notifyAll() are equivalent, but notify() is fragile: if another thread ever waited on the same monitor, notify() could wake the wrong one. Switch to notifyAll(). It wakes the same single waiter now (no thundering herd), and the wait loop already re-checks its conditions (shouldStop / _notified / recomputed wait time), so any extra wake-ups are handled safely. --- persistit/core/src/main/java/com/persistit/IOTaskRunnable.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/persistit/core/src/main/java/com/persistit/IOTaskRunnable.java b/persistit/core/src/main/java/com/persistit/IOTaskRunnable.java index e30aa57f4f..5788cfc2d3 100644 --- a/persistit/core/src/main/java/com/persistit/IOTaskRunnable.java +++ b/persistit/core/src/main/java/com/persistit/IOTaskRunnable.java @@ -12,6 +12,7 @@ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. + * Portions Copyrighted 2026 3A Systems, LLC */ package com.persistit; @@ -60,7 +61,7 @@ Thread getThread() { synchronized void kick() { if (!_notified) { _notified = true; - notify(); + notifyAll(); } } From 7433a4af151b14b06f3a5260aee7fad9a240d421 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Thu, 9 Jul 2026 17:08:56 +0300 Subject: [PATCH 2/2] Fix flaky WarmupTest.testWarmup: compare resident page set, not slot order WarmupTest.testWarmup asserted that after a bufferinventory+bufferpreload restart every buffer slot holds the same page as before shutdown (getBufferCopy(i) before == getBufferCopy(i) after). That is not what warmup guarantees: preloadBufferInventory() sorts the recorded pages by read order (PageNode.READ_COMPARATOR) and reallocates buffers via the clock algorithm, so a page is reloaded into the pool but not necessarily into its original slot. The per-slot check only happened to pass when the allocation order coincided, and flaked on ubuntu-latest / JDK 17 (expected:<0> but was:<2> - an empty slot where the original held page 2) while passing on JDK 11/21/25/26. Assert the real invariant instead: every valid, recordable page (skipping temporary and lock volumes, matching recordBufferInventory) resident at shutdown is resident again after warmup, comparing the set of pages rather than their slot positions. A genuine failure to preload a page is still caught. --- .../test/java/com/persistit/WarmupTest.java | 51 ++++++++++++++----- 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/persistit/core/src/test/java/com/persistit/WarmupTest.java b/persistit/core/src/test/java/com/persistit/WarmupTest.java index 1d0141aae3..67666e0a8d 100644 --- a/persistit/core/src/test/java/com/persistit/WarmupTest.java +++ b/persistit/core/src/test/java/com/persistit/WarmupTest.java @@ -20,9 +20,10 @@ import org.junit.Test; +import java.util.HashSet; import java.util.Properties; +import java.util.Set; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; public class WarmupTest extends PersistitUnitTestCase { @@ -44,24 +45,48 @@ public void testWarmup() throws Exception { ex.clear().append(i).store(); } - final Buffer[] buff = new Buffer[100]; - for (int i = 0; i < pool.getBufferCount(); ++i) { - buff[i] = pool.getBufferCopy(i); - } + /* + * Warmup (bufferinventory + bufferpreload) guarantees that the pages + * resident in the pool at shutdown are read back into the pool on restart. + * It does not guarantee that a page returns to the same buffer slot: + * preloadBufferInventory() sorts the recorded pages by read order and + * reallocates buffers via the clock algorithm, so the slot index is not + * preserved. Compare the set of resident pages, not per-slot correspondence + * (the latter only happened to match and made the test flaky). + */ + final Set before = residentPages(pool); + assertTrue("Test setup should leave pages resident in the pool", !before.isEmpty()); ex = null; _persistit.close(); _persistit = new Persistit(_config); - ex = _persistit.getExchange("persistit", "WarmupTest", false); - pool = ex.getBufferPool(); - - for (int i = 0; i < pool.getBufferCount(); ++i) { - final Buffer bufferCopy = pool.getBufferCopy(i); - assertEquals(bufferCopy.getPageAddress(), buff[i].getPageAddress()); - assertEquals(bufferCopy.getPageType(), buff[i].getPageType()); - assertEquals(bufferCopy.getBufferSize(), buff[i].getBufferSize()); + pool = _persistit.getVolume("persistit").getStructure().getPool(); + + final Set after = residentPages(pool); + final Set missing = new HashSet(before); + missing.removeAll(after); + assertTrue("Warmup should preload every previously-resident page; missing=" + missing, + missing.isEmpty()); + } + + /** + * Collects an identity ({@code volume:page:type:size}) for every valid, + * recordable buffer currently resident in the pool. Mirrors the filtering in + * {@link BufferPool#recordBufferInventory}, which skips temporary and lock + * volumes, so the returned set is exactly what warmup is expected to reload. + */ + private static Set residentPages(final BufferPool pool) { + final Set pages = new HashSet(); + for (int i = 0; i < pool.getBufferCount(); i++) { + final Buffer b = pool.getBufferCopy(i); + final Volume volume = b.getVolume(); + if (b.isValid() && volume != null && !volume.isTemporary() && !volume.isLockVolume()) { + pages.add(volume.getName() + ':' + b.getPageAddress() + ':' + b.getPageType() + ':' + + b.getBufferSize()); + } } + return pages; } @Test