Best practice to immideately stop Thread

Viewed 105

I have context, which may be used to produce prognosis. Algorithm to make prognosis based on context isn't part of my question. But it is time-expensive, so I moved it out from main thread. In any time context may be changed. When this happens app should restart prognosis producing with new input. Prognosis related to old context will became invalid, so its calculation should be interupted.

For now I use code:

public class BackgroundTasksManager {

public static final BackgroundTasksManager shared = new BackgroundTasksManager();

private ExecutorService conceptResultsExecutor = null;

private BackgroundTasksManager() { }

public void startConceptResultsCalculation(ConceptResultsCalculationInput input,
                                           Consumer<ConceptResultsCalculationOutput> callback) {
    ThreadGroup callbackGroup = Thread.currentThread().getThreadGroup();
    if (conceptResultsExecutor != null)
        conceptResultsExecutor.shutdownNow();

    conceptResultsExecutor = Executors.newSingleThreadExecutor();
    conceptResultsExecutor.submit(() -> {
        ConceptResultsCalculationOutput output = ConceptResultsCombinator.possbileResults(input);
        Thread callbackThread = new Thread(callbackGroup, () -> callback.accept(output));
        callbackThread.start();
    });
}

So, when I need to start prognosis calculation, I shut down old Executor and create new one. This solution works, but is it valid, or hide some dangerous nuances?

2 Answers

To stop a thread - you either interrupt it, via a direct call to Thread::interrupt or, in your case, call shutDownNow, which will implicitly call that. Or you could have a volatile boolean stop flag, that some other thread needs to set and your thread needs to check.

If your threads do not respond to interrupts, that shutDownNow will not do anything, besides blocking future tasks to be scheduled on it.

You can use Thread::stop. The method is Deprecated from long time (since 1.2) but it is still present in Java 15. The problem of the method as stated in javadocs is that

This method is inherently unsafe. Stopping a thread with Thread.stop causes it to unlock all of the monitors that it has locked (as a natural consequence of the unchecked ThreadDeath exception propagating up the stack). If any of the objects previously protected by these monitors were in an inconsistent state, the damaged objects become visible to other threads, potentially resulting in arbitrary behavior. Many uses of stop should be replaced by code that simply modifies some variable to indicate that the target thread should stop running. The target thread should check this variable regularly, and return from its run method in an orderly fashion if the variable indicates that it is to stop running. If the target thread waits for long periods (on a condition variable, for example), the interrupt method should be used to interrupt the wait.

The problem is not only relative to the locking but also to any elaboration that will be abruptly interrupted in the middle living you data inconsistent. But you can use try finally to do same cleanup

void run() {
    try {
       runInner();
    }
    finally {
        //do same checks
        //this is executed also if stop() is called where execution was inside runInner()
    }

The method of inserting some checks

volatile boolean myThreadStopping=false;

void run() {
    // some code
    if (myThreadStopping) return;
    // some code
    if (myThreadStopping) return;
    for (...) {
       // some code
       if (myThreadStopping) return;
    }
}

has the advantage of controlling the exit point and it allows you to leave data and locks consistent when returning from you task. If there is any sleep in your code use can use Thread::interrupt after myThreadStopping=true.

The greatest limit of active checking is the blocking IO methods. If in your code, for example, you read from an InputStream of a Socket, Thread::interrupt will not help you to make execution go ahead to next check of myThreadStopping variable and the Thread will keep wait at read().

You can combine the two methods.

Related