Copilot commented on code in PR #204:
URL:
https://github.com/apache/maven-toolchains-plugin/pull/204#discussion_r4302573220
##########
src/test/java/org/apache/maven/plugins/toolchain/jdk/ToolchainDiscovererTest.java:
##########
@@ -51,4 +74,142 @@ void testDiscovery() {
assertTrue(persistedToolchains.getToolchains().stream()
.anyMatch(tc -> tc.getProvides().containsKey(CURRENT)));
}
+
+ @Test
+ void cacheUpdateDuringWriteRemainsPending() throws Exception {
+ String originalUserHome =
System.getProperty(ToolchainDiscoverer.USER_HOME);
+ System.setProperty(ToolchainDiscoverer.USER_HOME,
temporaryDirectory.toString());
+ try {
+ BlockingCache cache = new BlockingCache();
+ TestToolchainDiscoverer discoverer = new TestToolchainDiscoverer();
+ setField(discoverer, "cache", cache);
+
+ Path firstJdk = temporaryDirectory.resolve("jdk-1");
+ Path secondJdk = temporaryDirectory.resolve("jdk-2");
+ discoverer.getToolchainModel(firstJdk);
+ cache.blockNextSnapshot();
+
+ AtomicReference<Throwable> writerFailure = new AtomicReference<>();
+ AtomicReference<Throwable> updaterFailure = new
AtomicReference<>();
+ Thread writer = new Thread(() -> {
+ try {
+ invokeWriteCache(discoverer);
+ } catch (Throwable t) {
+ writerFailure.set(t);
+ }
+ });
+ writer.start();
+
+ assertTrue(cache.snapshotStarted.await(10, TimeUnit.SECONDS),
"cache snapshot did not start");
+ Thread updater = new Thread(() -> {
+ try {
+ discoverer.getToolchainModel(secondJdk);
+ } catch (Throwable t) {
+ updaterFailure.set(t);
+ }
+ });
+ updater.start();
+ assertTrue(discoverer.secondModelCreated.await(10,
TimeUnit.SECONDS), "cache update did not start");
+ cache.allowSnapshot.countDown();
Review Comment:
`secondModelCreated` is signalled before the updater inserts the model and
marks the cache dirty. If the updater pauses after signalling, releasing
`allowSnapshot` lets the writer finish first. Even the original implementation
then passes: the later insertion marks the cache dirty again, and the final
write persists the second JDK. Move the writer barrier after snapshot creation
and outside `cacheLock`, then wait for the updater to finish before resuming
persistence. Do not simply join the updater at the current barrier, since the
writer still holds the lock it needs. This will make the regression test
reliably exercise the lost-update ordering.
--
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]
