papegaaij commented on code in PR #1617:
URL: https://github.com/apache/wicket/pull/1617#discussion_r4097652363
##########
wicket-core-tests/src/test/java/org/apache/wicket/pageStore/AsynchronousPageStoreTest.java:
##########
@@ -271,7 +273,7 @@ public IManageablePage getPage(IPageContext context, int id)
assertEquals(null, asyncPageStore.getPage(context, pageId));
- store.destroy();
+ asyncPageStore.destroy();
semaphore.release();
Review Comment:
Destroying the `AsynchronousPageStore` before `semaphore.release()` can make
this test hang forever. It is a race, which is why CI passed; I reproduced the
hang locally, with `main` stuck in `Thread.join` at
`AsynchronousPageStore.destroy()`.
When the page saving thread picks up the first add before `removeAllPages`
runs, it ends up blocked in the delegate's `semaphore.acquire()`. From there:
1. `destroy()` interrupts the thread, and the delegate's `catch
(InterruptedException e) {}` swallows the interrupt, clearing the flag.
2. The `while (!Thread.interrupted())` loop in `PageAddingRunnable`
therefore carries on and returns to `queue.poll(...)`, so the thread never
exits.
3. `destroy()` calls `join()` without a timeout, so it waits forever and
`semaphore.release()` is never reached.
When the saving thread has not yet taken the first add, `removeAllPages`
clears both, the interrupt lands in `poll()`, and the thread exits cleanly.
That is the path the CI runs happened to take. The class is tagged `SLOW`, so
`-Pfast` skips it locally.
Releasing the semaphore first lets the blocked add complete, after which the
interrupt reaches the thread in `poll()` and `destroy()` returns:
```suggestion
semaphore.release();
asyncPageStore.destroy();
```
Restoring the interrupt in the delegate's catch
(`Thread.currentThread().interrupt();`) would also fix it, and makes the test
robust against the same ordering mistake later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
