ExecutorService - unable to stop thread from ServletContextListener when context is destroyed

Question

I start Thread from ServletContextListener when the context is initialized and tries to stop it when the context is destroyed. Grade:

 public enum BlinkLedTask { INSTANCE; private Logger logger = RpiLogger.getLogger(getClass()); private Task task; private ExecutorService service; private BlinkLedTask() { } public void run(String[] frequency) { stop(); task = new Task(frequency); service = Executors.newSingleThreadExecutor(RpiThreadFactory.INSTANCE); service.execute(task); } public void stop() { if(Objects.isNull(task) || Objects.isNull(service)) { return; } try { task.terminate(); service.shutdownNow(); } catch (Exception cause) { logger.error(cause.getMessage(), cause); } } private static class Task implements Runnable { private volatile boolean running = true; private String[] frequency; private volatile Logger logger = RpiLogger.getLogger(getClass()); private Task(String[] frequency) { this.frequency = frequency; } @Override public void run() { while(running && !Thread.interrupted()) { try { resetLed(); blinkLed(); } catch (Throwable cause) { logger.error(cause.getMessage(), cause); running = false; try { resetLed(); } catch (Throwable ignore) { } } } } private void resetLed() throws Exception { executeScript(Script.BLINK_LED_RESET); } private void blinkLed() throws Exception { executeScript(Script.BLINK_LED, new String[]{frequency[0], frequency[1], frequency[2]}); } private void executeScript(Script script, String... args) { ScriptExecutor scriptExecutor = new ScriptExecutor(ScriptExecutor.BASH, script); scriptExecutor.execute(true, args); } private void terminate() { logger.info("Stopping - " + Thread.currentThread().getName()); running = false; } } } 

This is a Singleton , and it runs the shell script before it stops. This class can be called from anywhere, so I need to stop the thread, if a shell script is currently running, before creating a new Thread .

For testing purposes, I executed the run() method of this class when the context is initialized and called stop() at the time of destruction.

I deleted the war file after deleting the run() code, I expected stop() to complete the task , but that is not the case.

I also tried another implementation of run() and stop() :

 public void run(String[] frequency) { stop(); task = new Task(frequency); Thread thread = RpiThreadFactory.INSTANCE.newThread(task); tasks.add(ImmutablePair.of(thread, task)); thread.start(); } public void stop() { for(ImmutablePair<Thread, Task> pair : tasks) { try { pair.right.terminate(); pair.left.join(); } catch (Exception ex) { } } } 

Here tasks are private ArrayList<ImmutablePair<Thread, Task>> tasks = new ArrayList<ImmutablePair<Thread,Task>>(); . ImmutablePair belongs to commons-lang3. But I got java.util.ConcurrentModificationException on the for for loop iteration. The reason I don't know about.

Update

When the server stop() works as expected. I am using Jetty.

Update

RpiThreadFactory :

 import java.util.concurrent.ThreadFactory; import java.util.concurrent.atomic.AtomicInteger; import org.apache.log4j.Logger; import com.edfx.rpi.app.utils.logger.RpiLogger; public enum RpiThreadFactory implements ThreadFactory { INSTANCE; private final AtomicInteger poolNumber = new AtomicInteger(1); private final Logger logger = RpiLogger.getLogger(getClass()); private final ThreadGroup threadGroup; private final AtomicInteger threadNumber = new AtomicInteger(1); private final String namePrefix; private RpiThreadFactory() { SecurityManager securityManager = System.getSecurityManager(); threadGroup = (securityManager != null) ? securityManager.getThreadGroup() : Thread.currentThread().getThreadGroup(); namePrefix = "RpiPool-" + poolNumber.getAndIncrement() + "-Thread-"; } public Thread newThread(Runnable runnable) { Thread thread = new Thread(threadGroup, runnable, namePrefix + threadNumber.getAndIncrement(), 0); thread.setPriority(Thread.NORM_PRIORITY); thread.setUncaughtExceptionHandler(new Thread.UncaughtExceptionHandler() { public void uncaughtException(Thread thread, Throwable cause) { logger.error(cause.getMessage(), cause); } }); return thread; } } 

ScriptExecutor :

 import java.io.BufferedReader; import java.io.InputStream; import java.io.InputStreamReader; import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Objects; import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; import com.edfx.rpi.app.utils.logger.RpiLogger; public class ScriptExecutor { private static final Logger LOGGER = RpiLogger.getLogger(ScriptExecutor.class); public static final String BASH = "/bin/bash"; private Script script; private Process process; private String output; private int exitValue; public ScriptExecutor(Script script) { this.script = script; } public void execute(boolean destroyProcess, String... args) throws ScriptNotExistException { if(!script.exists()) { throw new ScriptNotExistException(script.getScriptName() + " does not exists."); } try { List<String> commands = new ArrayList<>(); commands.add(BASH); commands.add(script.getAbsoultePath()); if(Objects.nonNull(args)) { commands.addAll(Arrays.asList(args)); } StringBuilder builder = new StringBuilder("Executing script: "); builder.append(script.getScriptName()); if(Objects.nonNull(args) && args.length > 0) { builder.append(" with parameters: "); builder.append(StringUtils.join(args, " ")); } LOGGER.info(builder.toString()); ProcessBuilder processBuilder = new ProcessBuilder(commands.toArray(new String[commands.size()])); process = processBuilder.start(); StringBuilder outputBuilder = new StringBuilder(); InputStream inputStream = process.getInputStream(); InputStreamReader inputStreamReader = new InputStreamReader(inputStream); BufferedReader bufferedReader = new BufferedReader(inputStreamReader); String line = StringUtils.EMPTY; while ((line = bufferedReader.readLine()) != null) { outputBuilder.append(line); outputBuilder.append("\n"); } process.waitFor(); exitValue = process.exitValue(); LOGGER.info("Process for: " + script.getScriptName() + " is executed. Exit value: " + exitValue); if(destroyProcess) { destroyProcess(); } output = outputBuilder.toString(); } catch (Exception cause) { throw new ScriptExecutionException(cause); } } public String getOutput() { return output; } public int getExitValue() { return exitValue; } public void destroyProcess() { if(Objects.nonNull(process)) { LOGGER.info("Process for: " + script.getScriptName() + " is destroyed."); process.destroy(); } } } 

goal

This is a web application running in the Jetty web container. The server installed in the embedded java hardware is turned on. How this hardware has an LED attached to it. The application accepts an external request, which can be REST and triggers an LED. Therefore, the LED may blink for any request; but it only performs one request at a time.

This is why I have a stop that stops a previously running process, if any. stop works for normal state.

But I saw that when the LED is blinking, and I did the deployment without stopping the server, the current thread does not stop. If I stop the server and turn around and start again, the running thread kills at this time.

The thread loop is in while and executes Process for native. This Process is a one-time task, so this Process does not cause the thread to be killed.

To reproduce the problem that I did, I created a thread when initializing the context and tried to kill it when it was destroyed. Now, if I write something in contextDestroyed , I see that they are being executed.

I don’t understand why stopping the server kills the thread, and not when redeploying.

Answer 1

You must call process.destroy () on the process instance returned by processBuilder.start (). In fact, what you do when you call BlinkLedTask.terminate () just sets some flag. You should call process.destroy () at this moment.

Below I present an example of how you can rewrite this.It does not include your ScriptExecutor class (of course, you can move your logic there and return the process instance to BlinkLedTask when blinkLed () is called).

The main difference here is that I save the reference to the process instance in the blinkLedProcess field, and when terminate () is called, I directly call process.destroy () to kill the process.

You wrote that "When the server shuts down, stop () works as expected. I am using Jetty." Yes indeed. This is because by calling processBuilder.start(); You create a subprocess of your main berth process. When you kill a berth, all of its sub-orders are also killed. If you are not killing the pier, you need to manually kill the subprocess by calling the destroy () method.

It should be something like:

 public enum BlinkLedTask { (...) private Process resetLedProcess; private Process blinkLedProcess; (...) private void blinkLed() throws Exception { String[] args = new String[] { frequency[0], frequency[1], frequency[2] }; List<String> commands = new ArrayList<>(); //commands.add(BASH); commands.add(script.getAbsoultePath()); if (Objects.nonNull(args)) { commands.addAll(Arrays.asList(args)); } StringBuilder builder = new StringBuilder("Executing script: "); builder.append(script.getAbsoultePath()); if (Objects.nonNull(args) && (args.length > 0)) { builder.append(" with parameters: "); builder.append(StringUtils.join(args, " ")); } ProcessBuilder processBuilder = new ProcessBuilder(commands.toArray(new String[commands.size()])); blinkLedProcess = processBuilder.start(); StringBuilder outputBuilder = new StringBuilder(); InputStream inputStream = blinkLedProcess.getInputStream(); InputStreamReader inputStreamReader = new InputStreamReader(inputStream); BufferedReader bufferedReader = new BufferedReader(inputStreamReader); String line = StringUtils.EMPTY; while ((line = bufferedReader.readLine()) != null) { outputBuilder.append(line); outputBuilder.append("\n"); } blinkLedProcess.waitFor(); int exitValue = blinkLedProcess.exitValue(); System.out.println( "Process for: " + Script.BLINK_LED.getAbsoultePath() + " is executed. Exit value: " + exitValue); } (...) private void terminate() { System.out.println("Stopping - " + Thread.currentThread().getName()); running = false; if (resetLedProcess != null) { resetLedProcess.destroy(); System.out.println("Destroyed reset process"); } if (blinkLedProcess != null) { blinkLedProcess.destroy(); System.out.println("Destroyed blink process"); } } (...) } 

Answer 2

First of all, you should use awaitTermination to wait after calling shutdownNow .

shutdownNow will interrupt your thread. Are you sure ScriptExecutor not stopping interrupts?

This may be the reason for this.

Also doing this with a SchedulerService seems redundant since you only use one thread.

You can start a new thread that you installed in the daemon thread (see What is a Daemon thread in Java? ), And it automatically closes when you exit your program.

Answer 3

What exactly are you trying to achieve? If this is the only thread that runs throughout the life cycle of your web application, I will just write my own context listener with a thread like this ...

 public class MyContextListener implements ServletContextListener { Thread myThread; Task myTask; @Override public void contextInitialized(ServletContextEvent sce) { myTask = new Task(); myThread = new Thread(myTask); myThread.start(); } @Override public void contextDestroyed(ServletContextEvent sce) { myTask.terminate(); myThread.interrupt(); myThread.join(); } } 

I wonder what exactly you want to achieve. Do you need only one thread running a series of scripts? Or are you hoping, at some point in the future, to have a multi-threaded application?

Any reason for servlet context? Could you just run this direct java application?