Skip to content

Commit

Permalink
Test: wait for netty threads in a JUnit ClassRule (#30763)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jaymode committed May 22, 2018
1 parent 79eee5b commit b344bd9
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.rules.ExternalResource;

Expand Down Expand Up @@ -162,24 +163,6 @@ public static void initDefaultSettings() {
public static void destroyDefaultSettings() {
SECURITY_DEFAULT_SETTINGS = null;
customSecuritySettingsSource = null;
// Wait for the network threads to finish otherwise there is the possibility that one of
// the threads lingers and trips the thread leak detector
try {
GlobalEventExecutor.INSTANCE.awaitInactivity(5, TimeUnit.SECONDS);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
} catch (IllegalStateException e) {
if (e.getMessage().equals("thread was not started") == false) {
throw e;
}
// ignore since the thread was never started
}

try {
ThreadDeathWatcher.awaitInactivity(5, TimeUnit.SECONDS);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
}

@Rule
Expand All @@ -203,6 +186,35 @@ protected void before() throws Throwable {
}
};

/**
* A JUnit class level rule that runs after the AfterClass method in {@link ESIntegTestCase},
* which stops the cluster. After the cluster is stopped, there are a few netty threads that
* can linger, so we wait for them to finish otherwise these lingering threads can intermittently
* trigger the thread leak detector
*/
@ClassRule
public static final ExternalResource STOP_NETTY_RESOURCE = new ExternalResource() {
@Override
protected void after() {
try {
GlobalEventExecutor.INSTANCE.awaitInactivity(5, TimeUnit.SECONDS);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
} catch (IllegalStateException e) {
if (e.getMessage().equals("thread was not started") == false) {
throw e;
}
// ignore since the thread was never started
}

try {
ThreadDeathWatcher.awaitInactivity(5, TimeUnit.SECONDS);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
}
};

@Before
//before methods from the superclass are run before this, which means that the current cluster is ready to go
public void assertXPackIsInstalled() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.rules.ExternalResource;

Expand Down Expand Up @@ -97,25 +98,6 @@ private static void tearDownRestClient() {
IOUtils.closeWhileHandlingException(restClient);
restClient = null;
}

// Wait for the network threads to finish otherwise there is the possibility that one of
// the threads lingers and trips the thread leak detector
try {
GlobalEventExecutor.INSTANCE.awaitInactivity(5, TimeUnit.SECONDS);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
} catch (IllegalStateException e) {
if (e.getMessage().equals("thread was not started") == false) {
throw e;
}
// ignore since the thread was never started
}

try {
ThreadDeathWatcher.awaitInactivity(5, TimeUnit.SECONDS);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
}

@Rule
Expand All @@ -130,6 +112,35 @@ protected void before() {
}
};

/**
* A JUnit class level rule that runs after the AfterClass method in {@link ESIntegTestCase},
* which stops the cluster. After the cluster is stopped, there are a few netty threads that
* can linger, so we wait for them to finish otherwise these lingering threads can intermittently
* trigger the thread leak detector
*/
@ClassRule
public static final ExternalResource STOP_NETTY_RESOURCE = new ExternalResource() {
@Override
protected void after() {
try {
GlobalEventExecutor.INSTANCE.awaitInactivity(5, TimeUnit.SECONDS);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
} catch (IllegalStateException e) {
if (e.getMessage().equals("thread was not started") == false) {
throw e;
}
// ignore since the thread was never started
}

try {
ThreadDeathWatcher.awaitInactivity(5, TimeUnit.SECONDS);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
}
};

@Before
//before methods from the superclass are run before this, which means that the current cluster is ready to go
public void assertXPackIsInstalled() {
Expand Down

0 comments on commit b344bd9

Please sign in to comment.