Java: Killing a thread launched as a Runnable by ExecutorService

Question

I have a system which launches workers when it receives a call from a webservice to do so. The workers are launched by an ExecutorService, the class being launched implements Runnable. However, if the worker times out, I am unable to actually kill the worker, which is leading to resource issues on my system.

public class MyClass implements Runnable {

    public void internalCall() {
        logger.info("B-1");
        //Some business code which may take too long
        // <...>
        logger.info("B-2");
    }

    public void launch() {
        // Wrapper
        Callable<Object> callable = new Callable<Object>() {
            @Override
            public Object call() throws Exception {
                internalCall();
                return null;
            }
        };

        // Submit
        ExecutorService executor = Executors.newSingleThreadExecutor();
        Future<Object> future = executor.submit(callable);

        try {
            // Wait
            future.get(1, TimeUnit.SECONDS);
        }
        catch (TimeoutException e) {
            logger.warn("Timeout");
        }
        finally {
            logger.info("A-1");
            executor.shutdownNow();
            future.cancel(true);
            logger.info("A-2");
        }
    }
}

If the worker times out, I would expect the following log message:

INFO | B-1
WARN | Timeout
INFO | A-1
INFO | A-2

Followed by the service remaining idle until another worker request comes in. However, despite calling shutdownNow() and cancel() on the ExecutorService and Future respectably, the worker continues:

INFO | B-1
WARN | Timeout
INFO | A-1
INFO | A-2
INFO | B-2

I've looked around and there are a number of other, similar questions about killing threads, with the general consensus being that you shouldn't do it. However, this is a class which may be extended, with the intent to override the internalCall() - meaning I can't rely on internalCall to police itself and check Thread.isInterrupted() or anything like that.

I'd like to just force kill things from the launch() method by attacking the furure or executor object.

Answer 1

I had the same setup using a SingleThreadExecutor and a Future. But as I have no control over the code I execute, I couldn't implement checks for Thread.interrupted().
(I could add them via bytecode transformation, but they would need to be placed before every single statement to prevent things like infinite loops from happening).

My solution is using a plain Thread object along with a TimerTask.

var executionThread = new ExecutionThread(/* params... */);
var timer = new Timer();
var timeOutTask = new TimeOutTask(executionThread, timer);
timer.schedule(timeOutTask, MAX_EXECUTION_TIME_SECONDS * 1_000);
executionThread.start();

try {
    // wait for the execution to finish:
    executionThread.join();
} catch (InterruptedException e) {
    // I throw an exception here for example, but this case will usually not happen
}

if (timeOutTask.isThreadTimedOut()) {
    // custom exception I publish to the client, you can handle it different though
    throw new ExecutionTimedOut(MAX_TEST_EXECUTION_TIME_SECONDS);
}

// the result that you would usually receive from the future.
return executionThread.getResult();

The custom thread class can look something like this:
(I use lombok, you can manually write out the constructors and getters as well)

@RequiredArgsConstructor
class ExecutionThread extends Thread {
    @Getter private Result result;

    private final Things i_need_for_execution;

    @Override
    public void run() {
        result = // execute the task here ...
    }
}

The timer can just be copied like this:

@RequiredArgsConstructor
class TimeOutTask extends TimerTask {
    private final Thread thread;
    private final Timer timer;
    @Getter private boolean threadTimedOut = false;

    @Override
    public void run() {
        if (thread != null && thread.isAlive()) {
            //noinspection deprecation
            thread.stop();
            timer.cancel();
            threadTimedOut = true;
        }
    }
}

I would have wished for a solution that doesn't involve the deprecated Thread::stop, but that's the only way I found.

Answer 2

N.B.

What if a thread doesn't respond to Thread.interrupt?

In some cases, you can use application specific tricks. For example, if a thread is waiting on a known socket, you can close the socket to cause the thread to return immediately. Unfortunately, there really isn't any technique that works in general. It should be noted that in all situations where a waiting thread doesn't respond to Thread.interrupt, it wouldn't respond to Thread.stop either. Such cases include deliberate denial-of-service attacks, and I/O operations for which thread.stop and thread.interrupt do not work properly. - Java Thread Primitive Deprecation

So what have we learned... In Java do not run third party code that you really can't trust in the same execution as your primary application. Also even if Thread.stop did work you still have a whole bunch of other things that are far worse than threads not checking interrupt status (ie code calling System.exit(0)).

What I recommend you do for third party code that you can't trust is either:

• Run the third party code as an evaluated language run by Java that you control the execution of. Some examples are: rules languages like Drools or logicless templating language like JMustache.

• Run the third party code in a separate execution and use your operating system to kill the process and IPC such as sockets for communication.

Answer 3

Firstly, future.cancel(true) does not kill the running thread. It only attempts to stop its execution in a "polite manner".

What it does it sends interrupt signal, more or less, same way as if you called yourthread.interrupt() somewhere in your code.

It will stop the processing only if the code inside the run() method checks for the interruption. So inside your internalCall() you need to check every now and then if the thread was not interrupted by calling Thread.interrupted() and stops the execution. The interrupt will also stop the sleep(), wait(), IO operations by throwing InterruptedExcepiton (that is why those methods throw such exception).

ExecutorService is using the same mechanism, so it will attempt to interrupt the running threads, but again, if your code does not check for such interruption the thread will continue running.

Due to various reasons, threads should not be simply killed (check java doc for thread.stop() method and why it is deprecated), and the "universal" way of informing the threads that maybe they should stop working is the interrupt signal.