Pi4J / pi4j-v1

DEPRECATED Java I/O library for Raspberry Pi (GPIO, I2C, SPI, UART)
http://www.pi4j.com
Apache License 2.0
1.3k stars 446 forks source link

pi4j-scheduled-executor-x keeps growing #495

Closed ganeshs4 closed 3 years ago

ganeshs4 commented 4 years ago

I have a setup which controls garage door based on the input received (open/close) from a web interface. The code is something like below:

public class ShutterRemoteControl implements Runnable {

@Override
    public void run() {
        GpioController gpioGarageController = GpioFactory.getInstance();
        GpioPinDigitalOutput garageControlOpenOrClosePin = gpioGarageController
                .provisionDigitalOutputPin(RaspiPin.GPIO_04);
        logger.info("Setting the pin to high-low i.e. open shutter on thread name: " + Thread.currentThread().getName());
        garageControlOpenOrClosePin.pulse(2000, PinState.HIGH); //the control needs high followed by low pulse to operate
        try {
            logger.info("Thread is now going to keep looping in while loop, thread name is: "
                    + Thread.currentThread().getName());
            while (!GarageModel.isShutterCloseCommand()) {
                Thread.currentThread().sleep(2000);
            }
            logger.info("Thread is out of the while loop, thread name is: " + Thread.currentThread().getName());
            logger.info("Setting the pin to pulse i.e. close shutter now");
            garageControlOpenOrClosePin.pulse(2000, PinState.HIGH); //shutter needs high-low pulse to operate
            gpioGarageController.unprovisionPin(garageControlOpenOrClosePin); //tried commenting this too, didn't help
        } catch (Exception e) {
            logger.info("Exception occurred.." + e);
        }

    }
}

The above is executed as below:

    @RequestMapping(value = "/garagercontrol/{openOrClose}", method = RequestMethod.GET)
    public void openOrCloseGarageShutter(@PathVariable String openOrClose) {
        if (openOrClose.equalsIgnoreCase("open")) {
            GarageModel.setShutterCloseCommand(false);
            Thread shutterRemoteControl = new Thread(new ShutterRemoteControl());
            shutterRemoteControl.start();
        }

        if (openOrClose.equalsIgnoreCase("close")) { 
                    GarageModel.setShutterCloseCommand(true);
                }
    }

Whenever the above is executed, there are 2 additional thread added to the JVM which are never terminated. The 2 new thread added are in WAITING state. Every time the garage door is commanded to be opened it adds 2 threads to WAITING state, same when close command is issued. I captured thread dumps and see below which seems to be getting added but never terminates (seems like thread pool executor - pi4j-scheduled-executor-x):

"pi4j-scheduled-executor-8" #54 prio=5 os_prio=0 cpu=0.54ms elapsed=181.70s tid=0x633c5c00 nid=0xaae waiting on condition  [0x5f5de000]
   java.lang.Thread.State: WAITING (parking)
    at jdk.internal.misc.Unsafe.park(java.base@11.0.7/Native Method)
    - parking to wait for  <0x6b1ff3c8> (a java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject)
    at java.util.concurrent.locks.LockSupport.park(java.base@11.0.7/LockSupport.java:194)
    at java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject.await(java.base@11.0.7/AbstractQueuedSynchronizer.java:2081)
    at java.util.concurrent.ScheduledThreadPoolExecutor$DelayedWorkQueue.take(java.base@11.0.7/ScheduledThreadPoolExecutor.java:1177)
    at java.util.concurrent.ScheduledThreadPoolExecutor$DelayedWorkQueue.take(java.base@11.0.7/ScheduledThreadPoolExecutor.java:899)
    at java.util.concurrent.ThreadPoolExecutor.getTask(java.base@11.0.7/ThreadPoolExecutor.java:1054)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(java.base@11.0.7/ThreadPoolExecutor.java:1114)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(java.base@11.0.7/ThreadPoolExecutor.java:628)
    at java.util.concurrent.Executors$PrivilegedThreadFactory$1$1.run(java.base@11.0.7/Executors.java:668)
    at java.util.concurrent.Executors$PrivilegedThreadFactory$1$1.run(java.base@11.0.7/Executors.java:665)
    at java.security.AccessController.doPrivileged(java.base@11.0.7/Native Method)
    at java.util.concurrent.Executors$PrivilegedThreadFactory$1.run(java.base@11.0.7/Executors.java:665)
    at java.lang.Thread.run(java.base@11.0.7/Thread.java:834)

This keeps growing. I even tried singleton using ENUM but it's the same behavior. Any clue, how to handle this?

ganeshs4 commented 4 years ago

Not sure if this same as - https://github.com/Pi4J/pi4j/issues/479 , looks different to me.

ganeshs4 commented 4 years ago

Looks like it's the Future<?> pulse(long duration, PinState pulseState); which is causing the above behavior. Wouldn't the future task thread stop after the task has been completed?

If updated the code to do something like this instead of pulse it helps (I understand I could also use the blocking pulse call - haven't tried that though):

Before: Which was causing threads to keep increasing on every operation

garageControlOpenOrClosePin.pulse(2000, PinState.HIGH); //the control needs high followed by low pulse to operate

Changed it to:

garageControlOpenOrClosePin.high();
Thread.sleep(2000);
garageControlOpenOrClosePin.low();
savageautomate commented 4 years ago

Does it grow past 25 threads?
The underlying implementation is using a ScheduledExecutorService with a MAX_THREADS_IN_POOL = 25;

See: https://github.com/Pi4J/pi4j/blob/cd8051d02949dc13da47b4b177c7dd1a0bd6a16b/pi4j-core/src/main/java/com/pi4j/concurrent/DefaultExecutorServiceFactory.java#L43-L53

If you don't like this many threads, you can implement your own ExecutorServiceFactory and set it using this static method: https://github.com/Pi4J/pi4j/blob/cd8051d02949dc13da47b4b177c7dd1a0bd6a16b/pi4j-core/src/main/java/com/pi4j/io/gpio/GpioFactory.java#L142-L151

savageautomate commented 4 years ago

Looks like it's the Future<?> pulse(long duration, PinState pulseState); which is causing the above behavior. Wouldn't the future task thread stop after the task has been completed?

Not necessarily. TASK != THREAD From what I understand the thread pool may grow up to the maximum number of fixed threads allowed. The threads may remain around and IDLE for future re-use by the ScheduledExecutorService.

This can have some adverse affects as noted in Issue #489.

We are proposing to change the implementation to a single thread in Pi4J version 1.3 and forward. This puts more responsibility on the user to handle event listeners promptly and not block other events in the pipeline waiting on the thread but should better keep events orderly and in sequence.

eitch commented 4 years ago

On my branch I have implemented a single threaded GPIO event dispatcher, thus resolving this issue. One can use it by setting the following:

import com.pi4j.concurrent.SingleThreadGpioExecutorServiceFactory;
...
GpioFactory.setExecutorServiceFactory(new SingleThreadGpioExecutorServiceFactory());

My branch is at https://github.com/eitch/pi4j/tree/develop/1.4

savageautomate commented 3 years ago

Closed. No further activity and v1.4 switches to a single thread GPIO event dispatcher.