Skip to content

Commit b1d87d6

Browse files
committed
Test: wait for netty threads in a JUnit ClassRule (#30763)
This commit changes the wait for a few netty threads to wait for these threads to complete after the cluster has stopped. Previously, we were waiting for these threads before the cluster was actually stopped; the cluster is stopped in an AfterClass method of ESIntegTestCase, while the wait was performed in the AfterClass of a class that extended ESIntegTestCase, which is always executed before the AfterClass of ESIntegTestCase. Now, the wait is contained in an ExternalResource ClassRule that implements the waiting for the threads to terminate in the after method. This rule is executed after the AfterClass method in ESIntegTestCase. The same fix has also been applied in SecuritySingleNodeTestCase. Closes #30563
1 parent c0d7aa2 commit b1d87d6

File tree

2 files changed

+60
-37
lines changed

2 files changed

+60
-37
lines changed

x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java

Lines changed: 30 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import org.junit.AfterClass;
4646
import org.junit.Before;
4747
import org.junit.BeforeClass;
48+
import org.junit.ClassRule;
4849
import org.junit.Rule;
4950
import org.junit.rules.ExternalResource;
5051

@@ -162,24 +163,6 @@ public static void initDefaultSettings() {
162163
public static void destroyDefaultSettings() {
163164
SECURITY_DEFAULT_SETTINGS = null;
164165
customSecuritySettingsSource = null;
165-
// Wait for the network threads to finish otherwise there is the possibility that one of
166-
// the threads lingers and trips the thread leak detector
167-
try {
168-
GlobalEventExecutor.INSTANCE.awaitInactivity(5, TimeUnit.SECONDS);
169-
} catch (InterruptedException e) {
170-
Thread.currentThread().interrupt();
171-
} catch (IllegalStateException e) {
172-
if (e.getMessage().equals("thread was not started") == false) {
173-
throw e;
174-
}
175-
// ignore since the thread was never started
176-
}
177-
178-
try {
179-
ThreadDeathWatcher.awaitInactivity(5, TimeUnit.SECONDS);
180-
} catch (InterruptedException e) {
181-
Thread.currentThread().interrupt();
182-
}
183166
}
184167

185168
@Rule
@@ -203,6 +186,35 @@ protected void before() throws Throwable {
203186
}
204187
};
205188

189+
/**
190+
* A JUnit class level rule that runs after the AfterClass method in {@link ESIntegTestCase},
191+
* which stops the cluster. After the cluster is stopped, there are a few netty threads that
192+
* can linger, so we wait for them to finish otherwise these lingering threads can intermittently
193+
* trigger the thread leak detector
194+
*/
195+
@ClassRule
196+
public static final ExternalResource STOP_NETTY_RESOURCE = new ExternalResource() {
197+
@Override
198+
protected void after() {
199+
try {
200+
GlobalEventExecutor.INSTANCE.awaitInactivity(5, TimeUnit.SECONDS);
201+
} catch (InterruptedException e) {
202+
Thread.currentThread().interrupt();
203+
} catch (IllegalStateException e) {
204+
if (e.getMessage().equals("thread was not started") == false) {
205+
throw e;
206+
}
207+
// ignore since the thread was never started
208+
}
209+
210+
try {
211+
ThreadDeathWatcher.awaitInactivity(5, TimeUnit.SECONDS);
212+
} catch (InterruptedException e) {
213+
Thread.currentThread().interrupt();
214+
}
215+
}
216+
};
217+
206218
@Before
207219
//before methods from the superclass are run before this, which means that the current cluster is ready to go
208220
public void assertXPackIsInstalled() {

x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySingleNodeTestCase.java

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.junit.AfterClass;
2727
import org.junit.Before;
2828
import org.junit.BeforeClass;
29+
import org.junit.ClassRule;
2930
import org.junit.Rule;
3031
import org.junit.rules.ExternalResource;
3132

@@ -97,25 +98,6 @@ private static void tearDownRestClient() {
9798
IOUtils.closeWhileHandlingException(restClient);
9899
restClient = null;
99100
}
100-
101-
// Wait for the network threads to finish otherwise there is the possibility that one of
102-
// the threads lingers and trips the thread leak detector
103-
try {
104-
GlobalEventExecutor.INSTANCE.awaitInactivity(5, TimeUnit.SECONDS);
105-
} catch (InterruptedException e) {
106-
Thread.currentThread().interrupt();
107-
} catch (IllegalStateException e) {
108-
if (e.getMessage().equals("thread was not started") == false) {
109-
throw e;
110-
}
111-
// ignore since the thread was never started
112-
}
113-
114-
try {
115-
ThreadDeathWatcher.awaitInactivity(5, TimeUnit.SECONDS);
116-
} catch (InterruptedException e) {
117-
Thread.currentThread().interrupt();
118-
}
119101
}
120102

121103
@Rule
@@ -130,6 +112,35 @@ protected void before() {
130112
}
131113
};
132114

115+
/**
116+
* A JUnit class level rule that runs after the AfterClass method in {@link ESIntegTestCase},
117+
* which stops the cluster. After the cluster is stopped, there are a few netty threads that
118+
* can linger, so we wait for them to finish otherwise these lingering threads can intermittently
119+
* trigger the thread leak detector
120+
*/
121+
@ClassRule
122+
public static final ExternalResource STOP_NETTY_RESOURCE = new ExternalResource() {
123+
@Override
124+
protected void after() {
125+
try {
126+
GlobalEventExecutor.INSTANCE.awaitInactivity(5, TimeUnit.SECONDS);
127+
} catch (InterruptedException e) {
128+
Thread.currentThread().interrupt();
129+
} catch (IllegalStateException e) {
130+
if (e.getMessage().equals("thread was not started") == false) {
131+
throw e;
132+
}
133+
// ignore since the thread was never started
134+
}
135+
136+
try {
137+
ThreadDeathWatcher.awaitInactivity(5, TimeUnit.SECONDS);
138+
} catch (InterruptedException e) {
139+
Thread.currentThread().interrupt();
140+
}
141+
}
142+
};
143+
133144
@Before
134145
//before methods from the superclass are run before this, which means that the current cluster is ready to go
135146
public void assertXPackIsInstalled() {

0 commit comments

Comments
 (0)