java-native/jssc

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

hiddenalpha opened this issue · 1 comments

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'.

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