[ML] Fix race condition when closing time checker (#43098)

The tests for the ML TimeoutChecker rely on threads
not being interrupted after the TimeoutChecker is
closed.  This change ensures this by making the
close() and setTimeoutExceeded() methods synchronized
so that the code inside them cannot execute
simultaneously.

Fixes #43097
This commit is contained in:
David Roberts 2019-06-11 16:38:35 +01:00
parent b17fbe2933
commit d3136f99e6
1 changed files with 12 additions and 2 deletions

View File

@ -44,6 +44,7 @@ public class TimeoutChecker implements Closeable {
private final TimeValue timeout; private final TimeValue timeout;
private final Thread checkedThread; private final Thread checkedThread;
private final ScheduledFuture<?> future; private final ScheduledFuture<?> future;
private boolean isClosed; // only accessed within synchronized methods
private volatile boolean timeoutExceeded; private volatile boolean timeoutExceeded;
/** /**
@ -66,9 +67,13 @@ public class TimeoutChecker implements Closeable {
* Stops the timer if running. * Stops the timer if running.
*/ */
@Override @Override
public void close() { public synchronized void close() {
if (isClosed) {
return;
}
FutureUtils.cancel(future); FutureUtils.cancel(future);
timeoutCheckerWatchdog.remove(checkedThread); timeoutCheckerWatchdog.remove(checkedThread);
isClosed = true;
} }
/** /**
@ -104,7 +109,12 @@ public class TimeoutChecker implements Closeable {
} }
} }
private void setTimeoutExceeded() { private synchronized void setTimeoutExceeded() {
// Even though close() cancels the timer, it's possible that it can already be running when close()
// is called, so this check prevents the effects of this method occurring after close() returns
if (isClosed) {
return;
}
timeoutExceeded = true; timeoutExceeded = true;
timeoutCheckerWatchdog.interruptLongRunningThreadIfRegistered(checkedThread); timeoutCheckerWatchdog.interruptLongRunningThreadIfRegistered(checkedThread);
} }