Infinite-loop in readBytes when another thread calls close just in the right moment

[hiddenalpha]

Method

Java_jssc_SerialNativeInterface_readBytes(JNIEnv*, jobject, jlong, jint);

runs into infinite-loop if another thread manages to call close right in the correct moment (race-condition).

Tested with v2.9.2

(Has some similarities with #94)

How to Reproduce

This is hard to reproduce as this is a race-condition and we have an isOpen-check just before the read in SerialPort. So Thread-B has to call close EXACTLY while Thread-A just has passed the isOpen-check. I see two ways to reach this state. The 1st is circumventing SerialPort (as my reproducer does) and the other is using a java debugger to control when which thread does what (see explanation further down).

Writing a reproducer is simpler using the 1st approach. Because interacting with the debugger within our reproducer potentially makes reproducer ways too complex.

• Prepare the reproducer below
• Then the reproducer (for simplicity) calls close by himself. Remind: In the real-world scenario, this call would be performed by another thread somewhere else.
• Consult the CPU monitor. The JVM already uses all the CPU time.

This is due to missing error-handling in the native code. Eg by ignoring errors of select and read.

Reproducer:

import jssc.SerialNativeInterface;

class InfiniteLoopWhenReadBytesOnClosedPort
{

    private static final String portName = "/dev/ttyACM0";
    private static final int baudr = 9600;
    private static final boolean doExclusiveLock = false;
    private long fd;

    public static void main( String[] args )
    {
        new InfiniteLoopWhenReadBytesOnClosedPort().reproduce();
    }

    private void reproduce()
    {
        // Setup
        SerialNativeInterface serialNativeInterface = new SerialNativeInterface();
        fd = serialNativeInterface.openPort( portName, doExclusiveLock );
        if( fd<0 ) throw new IllegalStateException( "openPort( "+ portName +" ) failed with code "+ fd +". Is the device plugged in?");
        serialNativeInterface.setParams( fd, baudr, 8, 1, 0, true, true, 0 );

        while(true){

            // Imagine an other thread is closing fd just after we passsed the
            // isOpen check in SerialPort.readBytes().
            serialNativeInterface.closePort( fd );

            // Read a few bytes.
            System.out.println( "readBytes( fd, 4 )" );
            byte[] bytes = serialNativeInterface.readBytes( fd, 4 );

            // Log them.
            System.out.print( "Got "+ bytes.length +" bytes:" );
            for( byte b : bytes ){
                System.out.print( String.format(" 0x%02X", b) );
            }
            System.out.print("\n");
        }
    }
}

But what about the isOpen-check you might ask

If we like to reproduce it through SerialPort (and the isOpen-check), we could to this by using a java debugger. Launch two threads, one for reading and another for calling close and prepare two breakpoints before launch. One where Thread-B is just about calling close and the other one where Thread-A just has passed the isOpen-check. Then we have to tell the debugger that in case of breakpoints other threads are allowed to continue execution.

As soon this is setup, Make sure Thread-A runs into breakpoint just after he checked isOpen is ok. Then let him wait there and ensure Thread-B can call (and return) from his close call. After close got called, let Thread-A continue and consult your CPU monitor to see the JVM eating up all the CPU time.

Note

90 tries to reduce the risk of this to happen by synchronizing SerialPort.readBytes and SerialPort.closePort. This way our Thread-B would not be able to enter 'close' while Thread-A is running inside 'readBytes'.

[factoritbv]

Probably the improvements that we suggest in #126 fixes your problems.