Skip to content

Commit

Permalink
Eliminate race condition in Tomcat's graceful shutdown
Browse files Browse the repository at this point in the history
There was a race condition between the thread that's waiting for
Tomcat to become inactive or the graceful shutdown to be aborted
and the thread that aborts the shutdown and stops Tomcat when the
grace period has elapsed. This race can lead to Tomcat appearing
to have become inactive before the abort of the shutdown is
noticed. When this happens, the result of the shutdown is reported
as IDLE when it should have been REQUESTS_ACTIVE. The consequences
of this are mostly benign although it does affect the log messages
that are emitted. It is also causing some of our graceful shutdown
tests to be flaky.

This commit eliminates the race condition by considering the
state of the aborted flag before logging and returning the result
of the shutdown.

Closes gh-39942
  • Loading branch information
wilkinsona committed Mar 15, 2024
1 parent c83f701 commit 81dc0cc
Showing 1 changed file with 23 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -67,26 +67,15 @@ private void doShutdown(GracefulShutdownCallback callback, CountDownLatch shutdo
List<Connector> connectors = getConnectors();
connectors.forEach(this::close);
shutdownUnderway.countDown();
try {
for (Container host : this.tomcat.getEngine().findChildren()) {
for (Container context : host.findChildren()) {
while (isActive(context)) {
if (this.aborted) {
logger.info("Graceful shutdown aborted with one or more requests still active");
callback.shutdownComplete(GracefulShutdownResult.REQUESTS_ACTIVE);
return;
}
Thread.sleep(50);
}
}
}

awaitInactiveOrAborted();
if (this.aborted) {
logger.info("Graceful shutdown aborted with one or more requests still active");
callback.shutdownComplete(GracefulShutdownResult.REQUESTS_ACTIVE);
}
catch (InterruptedException ex) {
Thread.currentThread().interrupt();
else {
logger.info("Graceful shutdown complete");
callback.shutdownComplete(GracefulShutdownResult.IDLE);
}
logger.info("Graceful shutdown complete");
callback.shutdownComplete(GracefulShutdownResult.IDLE);
}
finally {
shutdownUnderway.countDown();
Expand All @@ -106,6 +95,22 @@ private void close(Connector connector) {
connector.getProtocolHandler().closeServerSocketGraceful();
}

private void awaitInactiveOrAborted() {
try {
for (Container host : this.tomcat.getEngine().findChildren()) {
for (Container context : host.findChildren()) {
while (!this.aborted && isActive(context)) {
Thread.sleep(50);
}
}
}

}
catch (InterruptedException ex) {
Thread.currentThread().interrupt();
}
}

private boolean isActive(Container context) {
try {
if (((StandardContext) context).getInProgressAsyncCount() > 0) {
Expand Down

0 comments on commit 81dc0cc

Please sign in to comment.