Skip to content

Commit ff793d4

Browse files
authored
[Test] Ensure racing happens as expected (elastic#92757)
The test expects the loading work takes long enough so that all racing threads have time to enter the concurrency-controlled code region. However, it is possible the loading work is fast enough (or it might be other threads are too slow) and it finishes before all racing threads are ready. This PR makes the loading method wait for a signal before completing. The signal is sent once all racing threads are actually ready. Resolves: elastic#92745
1 parent b1bfce5 commit ff793d4

File tree

1 file changed

+27
-13
lines changed

1 file changed

+27
-13
lines changed

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/jwt/JwkSetLoaderTests.java

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import org.elasticsearch.test.ESTestCase;
1414
import org.elasticsearch.xpack.core.security.authc.RealmConfig;
1515
import org.elasticsearch.xpack.core.security.authc.jwt.JwtRealmSettings;
16+
import org.mockito.Mockito;
1617

1718
import java.io.IOException;
1819
import java.nio.charset.StandardCharsets;
@@ -28,14 +29,13 @@
2829
import static org.hamcrest.Matchers.sameInstance;
2930
import static org.mockito.Mockito.doAnswer;
3031
import static org.mockito.Mockito.mock;
32+
import static org.mockito.Mockito.never;
3133
import static org.mockito.Mockito.spy;
32-
import static org.mockito.Mockito.times;
3334
import static org.mockito.Mockito.verify;
3435
import static org.mockito.Mockito.when;
3536

3637
public class JwkSetLoaderTests extends ESTestCase {
3738

38-
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/92745")
3939
public void testConcurrentReloadWillBeQueuedAndShareTheResults() throws IOException, InterruptedException {
4040
final Path tempDir = createTempDir();
4141
final Path path = tempDir.resolve("jwkset.json");
@@ -53,23 +53,37 @@ public void testConcurrentReloadWillBeQueuedAndShareTheResults() throws IOExcept
5353
.mapToObj(i -> new PlainActionFuture<Tuple<Boolean, JwkSetLoader.JwksAlgs>>())
5454
.toList();
5555

56-
final var threadsCountDown = new CountDownLatch(nThreads);
56+
// Start the first thread for reloading
57+
// Ensure it is inside the actual loading method and make it wait there to simulate slow processing
58+
final var loadingLatch = new CountDownLatch(1);
5759
final var readyLatch = new CountDownLatch(1);
5860
doAnswer(invocation -> {
59-
// Mark the thread to be ready when it enters the concurrency controlling area
60-
threadsCountDown.countDown();
61-
// Wait till the ready flag to start the racing
61+
loadingLatch.countDown();
6262
assertThat(readyLatch.await(10, TimeUnit.SECONDS), is(true));
63-
return invocation.callRealMethod();
64-
}).when(jwkSetLoader).getFuture();
63+
invocation.callRealMethod();
64+
return null;
65+
}).when(jwkSetLoader).loadInternal(anyActionListener());
6566

66-
// Start the threads and toggle flags to start racing
67-
IntStream.range(0, nThreads).forEach(i -> new Thread(() -> jwkSetLoader.reload(futures.get(i))).start());
67+
new Thread(() -> jwkSetLoader.reload(futures.get(0))).start();
68+
assertThat(loadingLatch.await(10, TimeUnit.SECONDS), is(true));
69+
70+
// Start rest of the threads for racing and ensure they are all through the concurrency controlling area
71+
Mockito.reset(jwkSetLoader);
72+
final var threadsCountDown = new CountDownLatch(nThreads - 1);
73+
doAnswer(invocation -> {
74+
final Object result = invocation.callRealMethod();
75+
threadsCountDown.countDown();
76+
return result;
77+
}).when(jwkSetLoader).getFuture();
78+
IntStream.range(1, nThreads).forEach(i -> new Thread(() -> jwkSetLoader.reload(futures.get(i))).start());
6879
assertThat(threadsCountDown.await(10, TimeUnit.SECONDS), is(true));
80+
81+
// Notify the first thread to finish the loading work
6982
readyLatch.countDown();
7083

71-
// All concurrent reloading calls will get the same result and the actual reloading work will only happen once
72-
futures.forEach(future -> assertThat(future.actionGet(), sameInstance(futures.get(0).actionGet())));
73-
verify(jwkSetLoader, times(1)).loadInternal(anyActionListener());
84+
// All concurrent reloading calls will get the same result from the first thread and skip the actual loading work
85+
final Tuple<Boolean, JwkSetLoader.JwksAlgs> tuple = futures.get(0).actionGet();
86+
futures.subList(1, nThreads).forEach(future -> assertThat(future.actionGet(), sameInstance(tuple)));
87+
verify(jwkSetLoader, never()).loadInternal(anyActionListener());
7488
}
7589
}

0 commit comments

Comments
 (0)