Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
hiddenalpha opened this issue Jul 29, 2021 · 1 comment

Comments

@hiddenalpha
Copy link

hiddenalpha commented Jul 29, 2021

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants