Pi4j genereates lots of threads

[menazord]

I have some doubts using Pi4J.

I'm currently using my Raspberry Pi (Model B) to communicate via USB to my Arduino Uno board. For this purpose I'm using Pi4j, thorugh serial port /dev/ttyACM0

My serial communication code looks like this (this function is executed every 5 minutes)

public DeviceData readData() throws DeviceException{

    try{
        DeviceData data = new DeviceData();

        if (serialPort == null || "".equals(serialPort))
            serialPort = Serial.DEFAULT_COM_PORT;

        serial = SerialFactory.createInstance();
        serial.addListener(
                event -> {
                    // print out the data received to the console
                    String payload;
                    try {
                        payload = event.getAsciiString();
                        LOGGER.debug("Arduino said :" + payload);
                    } catch (IOException ioe) {
                        throw new RuntimeException(ioe);
                    }

                }

        );

        LOGGER.info("Attempting connection to " + serialPort + " , baud rate " + baudRate);

        serial.open(serialPort, baudRate);

        if (serial.isOpen()){
            LOGGER.info("Connected");
            // send request, get data

            LOGGER.debug("Probing");
            serial.write('D');
            Thread.sleep(3000);

            LOGGER.debug("Requesting FREE MEMORY");
            serial.write('F');
            Thread.sleep(1500);

            serial.close();
            SerialFactory.shutdown();

            LOGGER.info("Disconnected");

        }

        return data;

    } catch (IOException e) {
        throw new DeviceException("Error while connecting to device.", e);
    } catch (Exception e) {
        throw new DeviceException("Unexpected error while connecting to device.", e);
    } finally {

    }
}

Even when using both serial.close() and SerialFactory.shutdown(), I can see on Java Mission Control that there are a lot of waiting threads with the same name (pi4j-single-executor-0) and increasing over time.

Is this a memory leak ?
Should I reuse SerialFactory object ?
Should I also reuse Serial object ?

[savageautomate]

Yes this does look like a bug. It looks like there is an executor service that gets created each time you get a serial instance from the factory and it is not getting shutdown when you make your call to SerialFactory.shutdown();

While this is a bug we need to look into, I also believe your program should only create one Serial instance and simply re-use it as needed. Calling serial.close() will not dispose the object ... you can just re-open the serial connection anytime you need. With a serial instance defined you also only need to create your serial listener once. This should make the code a bit more optimized.

[menazord]

Hi @savageautomate

Thank you for your input!

Let me try to put those optimizations in place and I'll post back the results I get.

[savageautomate]

I also noticed that you are writing some serial data and then immediately closing the connection. This may not be enough time to read the response back from the connected Arduino Uno.

            LOGGER.debug("Requesting FREE MEMORY");
            serial.write('F');
            Thread.sleep(1500);

            serial.close();
            SerialFactory.shutdown();

Also, you mentioned USB and you also mentioned /dev/ttyACM0. I assume the USB is on your Arduino and your are using a USB to serial breakout cable and connecting it to the TX and RX pins on the Raspberry Pi GPIO header.

Else, if you are trying to use the USB on the RPi to communicate via serial, then /dev/ttyACM0 is not the right serial port address.

[savageautomate]

Nevermind on the timing comment ... I completely overlooked the sleep you have in the code.

[menazord]

Hi @savageautomate

Turns out I am able to get response from arduino every time using this code

Connection between Pi and Arduino is made using USB A/B cable from Raspberry Pi USB port to Arduino Uno USB port.

This USB device shows up as /dev/ttyACM0 on Raspbian

/dev/ttyACM0 - Arduino__www.arduino.cc__0043_9523234383335111F0E0

This is the output

2017-09-05 11:25:00.073  INFO 1379 --- [pool-2-thread-1] c.f.gardenwatch.device.pi.RaspberryPi    : Connected
2017-09-05 11:25:00.078 DEBUG 1379 --- [pool-2-thread-1] c.f.gardenwatch.device.pi.RaspberryPi    : Probing
2017-09-05 11:25:09.106 DEBUG 1379 --- [pool-2-thread-1] c.f.gardenwatch.device.pi.RaspberryPi    : Requesting FREE MEMORY
2017-09-05 11:25:09.173 DEBUG 1379 --- [ngle-executor-0] c.f.gardenwatch.device.pi.RaspberryPi    : Arduino said :FMEM1587
2017-09-05 11:25:10.615  INFO 1379 --- [pool-2-thread-1] c.f.gardenwatch.device.pi.RaspberryPi    : Disconnected

Note that thread "pi4j-single-executor-0" also seeems to be related with the SerialDataEventListener output (4th line)

[savageautomate]

Interesting, I thought that /dev/ttyACM0 was reserved for the internal hardware UART. All of the USB to serial interfaces I have used in the past all showed up as /dev/ttyUSBxxxxxx

[savageautomate]

Nevermind, internal UART is ttyAMA0 It funny how one little letter make a world of difference :-)

[savageautomate]

There is a single thread executor service created for the serial instance and his job to is dispatch serial data events out to serial data listeners on this dedicated (event) thread. The intent here is to prevent blocking on the main thread that is reading the serial data from the native C implementation.

I'm not sure why the shutdown call was not being honored properly. In looking at the code, I really don't like some of the way its implemented, so I may refactor it a bit.

Just to make sure .. are you running version 1.2-SNAPSHOT?

Thanks, Robert

[menazord]

Hi Robert

True, one letter, different worlds :)

I was also checking the code last night to understand how the executor service works but did not find a reason why shutdown did not work as expected.

I'm currently using version 1.1 , as this is the latest available on Maven (I use maven repositories for my Gradle builds)

OH, one thing:

If /dev/ttyACM0 is the right port, maybe you should consider reflecting this on com.pi4j.io.serial.RaspberryPiSerial.DEFAULT_COM_PORT

[savageautomate]

1.2 snapshot has a lot of fixes and I'm working on getting it released soon. Any changes/fixes I make will be in these builds.

You can easily switch to include the snapshot version in your Maven build. See the snapshot builds section here for the additional repo you would need to include:
http://pi4j.com/download.html

com.pi4j.io.serial.RaspberryPiSerial.DEFAULT_COM_PORT is correct, it should be /dev/ttyAMA0. This is the serial port for the hardware pins RX and TX on the 40 pin header. I just misread when you posted /dev/ttyACM0 I thought I was reading /dev/ttyAMA0 and that's why I was questioning the USB connection. I had not seen a serial port named ttyACMxxx before on the RaspberryPi. So it was just unfamiliar and me mis-reading the actual text.

[menazord]

Great!

Let me try with the SNAPSHOT and I'll share my results

Thank you!

[menazord]

I think I have a clue

private static boolean isshutdown = false;
This static boolean field on SerialFactory changes to true on shutdown, but it does not changes to false again if you create a new instance.

This boolean controls shutting down executors, so when you create a new Serial using SerialFactory.createInstance() , if you shutdown the Factorty again, does not shutdown executors.

This is why SerialFactory.shutdown() is always able to terminate the first executor only.

I think adding isshutdown = false on SerialFactory.createInstance() might do it.

Hope this helps, I'm using 1.2.SNAPSHOT now

[menazord]

Also reusing SerialFactory should be a workaround for this

[menazord]

OK... I was able to sort this out, performing several tweaks to code

1. Reusing the same Serial object, opening and closing it on each iteration
2. For some reason, SerialDataEventListener had to be removed before closing Serial, and added again before opening.
3. I'm not using SerialFactory.shutdown anymore
4. I'm using version 1.2,SNAPSHOT but I guess results may not differ from 1.1

Results are really good in terms of less JVM memory. Also only 1 thread is running after 50 iterations

Thanks a lot for your suggestions Robert, and congratulations for your work on this project

[savageautomate]

Good catch on the private static boolean isshutdown = false;.
I'll patch this today.

[savageautomate]

A new snapshot build will include this fix in about an hour.

[menazord]

Excellent news, thanks a lot