From b2fc0bc1395026280977a5ee58dd5ea3c66e2f98 Mon Sep 17 00:00:00 2001
From: Michael Nitschinger <michael@nitschinger.at>
Date: Tue, 16 Jan 2024 19:09:03 +0100
Subject: [PATCH] ClientEffectiveStrategyTest: Attempt at fixing flakyness
 (#2804)

Based on initial analysis the ClientInvokingThreadRecorder's state
gets written and read from multiple threads, but not all of it is
properly thread-safe. The invokingThreads and errors use thread-safe
datastructures, but the offloadPoints do not.

Since the offloadPoints are on the critical path for the flaky
test failures, let's start by making them thread safe and check
if the errors still show up.

If they still persist, there might be more coordination needed
between the recording and checking threads as well.
---
 .../http/netty/ClientEffectiveStrategyTest.java          | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/ClientEffectiveStrategyTest.java b/servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/ClientEffectiveStrategyTest.java
index 7e459f6f80..52a8ef4a53 100644
--- a/servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/ClientEffectiveStrategyTest.java
+++ b/servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/ClientEffectiveStrategyTest.java
@@ -65,10 +65,12 @@
 import java.net.InetSocketAddress;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.EnumSet;
 import java.util.List;
 import java.util.Objects;
 import java.util.Queue;
+import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.LinkedBlockingQueue;
@@ -478,9 +480,10 @@ private static Buffer content(HttpExecutionContext ctx) {
 
     private static final class ClientInvokingThreadRecorder implements StreamingHttpClientFilterFactory {
 
-        private Thread applicationThread = Thread.currentThread();
-        private HttpExecutionStrategy expectedStrategy;
-        private final EnumSet<ClientOffloadPoint> offloadPoints = EnumSet.noneOf(ClientOffloadPoint.class);
+        private volatile Thread applicationThread = Thread.currentThread();
+        private volatile HttpExecutionStrategy expectedStrategy;
+        private final Set<ClientOffloadPoint> offloadPoints =
+                Collections.synchronizedSet(EnumSet.noneOf(ClientOffloadPoint.class));
         private final ConcurrentMap<ClientOffloadPoint, String> invokingThreads = new ConcurrentHashMap<>();
         private final Queue<Throwable> errors = new LinkedBlockingQueue<>();