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.