[RFZDEV-30379] Unblock main thread if init fails

When the monitor thread encounters an error during initialization, the main thread hangs indefinitely because the monitor thread will bail without ever releasing the synchronization lock and without signaling the `Condition` on which the main thread is waiting. Unblock the main thread by adding the signal/unlock sequence to the failure path. Additionally, throw a Java exception so we learn more about what causes the error. Co-authored-by: Jan-Niklas Dornbach <[email protected]> Co-authored-by: Martin Mungard <[email protected]> Co-authored-by: Richard L. Jackson <[email protected]>
dbsystel · Jan 6, 2025 · 67c208b · 67c208b
1 parent ef2aeb3
commit 67c208b
Show file tree

Hide file tree

Showing 7 changed files with 157 additions and 20 deletions.
diff --git a/pom.xml b/pom.xml
@@ -4,7 +4,7 @@
   <modelVersion>4.0.0</modelVersion>
   <groupId>de.bahn</groupId>
   <artifactId>nrjavaserial</artifactId>
-  <version>5.2.1+dbsystel2</version>
+  <version>5.2.1+dbsystel3</version>
   <name>NRJavaSerial</name>
   <description>A fork of the RXTX library with a focus on ease of use and embeddability in other libraries.</description>
   <url>http://neuronrobotics.com</url>

diff --git a/src/main/c/include/SerialImp.h b/src/main/c/include/SerialImp.h
@@ -416,6 +416,7 @@ printf("%8li sec : %8li usec\n", enow.tv_sec - snow.tv_sec, enow.tv_sec - snow.t
 #define OUT_OF_MEMORY "java/lang/OutOfMemoryError"
 #define IO_EXCEPTION "java/io/IOException"
 #define PORT_IN_USE_EXCEPTION "gnu/io/PortInUseException"
+#define MONITOR_INITIALIZATION_EXCEPTION "gnu/io/MonitorInitializationException"
 
 /* some popular releases of Slackware do not have SSIZE_MAX */
 

diff --git a/src/main/c/resources/native/linux/x86_64/libNRJavaSerial.so b/src/main/c/resources/native/linux/x86_64/libNRJavaSerial.so
diff --git a/src/main/c/src/SerialImp.c b/src/main/c/src/SerialImp.c
@@ -3823,7 +3823,8 @@ int lock_monitor_thread(JNIEnv *env, jobject jobj)
 }
 
 /**
- * Signal that the monitor thread is ready for work.
+ * Signal that the monitor thread has finished initialization and
+ * is either ready for work or has thrown a Java exception.
  *
  * In order to signal the condition, the current thread must already hold the
  * write lock.
@@ -3832,35 +3833,35 @@ int lock_monitor_thread(JNIEnv *env, jobject jobj)
  * @param [in] obj an RXTXPort instance
  * @return 0 on success; 1 on error
  */
-int signal_monitor_thread_ready(JNIEnv *env, jobject jobj)
+int signal_monitor_thread_init_completed(JNIEnv *env, jobject jobj)
 {
-	jfieldID monitorThreadReadyField = (*env)->GetFieldID(
+	jfieldID monitorThreadInitCompletedField = (*env)->GetFieldID(
 		env,
 		(*env)->GetObjectClass(env, jobj),
-		"monitorThreadReady",
+		"monitorThreadInitCompleted",
 		"Ljava/util/concurrent/locks/Condition;");
 	if ((*env)->ExceptionCheck(env)) {
 		return 1;
 	}
 
-	jobject monitorThreadReady = (*env)->GetObjectField(
+	jobject monitorThreadInitCompleted = (*env)->GetObjectField(
 		env,
 		jobj,
-		monitorThreadReadyField);
+		monitorThreadInitCompletedField);
 	if ((*env)->ExceptionCheck(env)) {
 		return 1;
 	}
 
 	jmethodID signal = (*env)->GetMethodID(
 		env,
-		(*env)->GetObjectClass(env, monitorThreadReady),
+		(*env)->GetObjectClass(env, monitorThreadInitCompleted),
 		"signal",
 		"()V");
 	if ((*env)->ExceptionCheck(env)) {
 		return 1;
 	}
 
-	(*env)->CallVoidMethod(env, monitorThreadReady, signal);
+	(*env)->CallVoidMethod(env, monitorThreadInitCompleted, signal);
 	if ((*env)->ExceptionCheck(env)) {
 		return 1;
 	}
@@ -4250,6 +4251,7 @@ int initialise_event_info_struct( struct event_info_struct *eis )
 	jobject jobj = *eis->jobj;
 	JNIEnv *env = eis->env;
 	struct event_info_struct *index = master_index;
+	char message[28];
 
 	if ( eis->initialised == 1 )
 		goto end;
@@ -4293,8 +4295,11 @@ int initialise_event_info_struct( struct event_info_struct *eis )
 
 	eis->send_event = (*env)->GetMethodID( env, eis->jclazz, "sendEvent",
 		"(IZ)Z" );
-	if(eis->send_event == NULL)
+	if(eis->send_event == NULL) {
+		throw_java_exception(env, MONITOR_INITIALIZATION_EXCEPTION,
+			"initialise_event_info_struct", "Unable to find `sendEvent`");
 		goto fail;
+	}
 end:
 	FD_ZERO( &eis->rfds );
 	if (eis->fd < FD_SETSIZE && eis->fd > 0) {
@@ -4304,7 +4309,10 @@ int initialise_event_info_struct( struct event_info_struct *eis )
 		eis->initialised = 1;
 		return( 1 );
 	} else {
-		// you can reduce this limitation only with migration to epool or something like that.
+		// you can reduce this limitation only with migration to poll or something like that.
+		snprintf(message, sizeof(message), "fd == %i out of range", eis->fd);
+		throw_java_exception(env, MONITOR_INITIALIZATION_EXCEPTION,
+			"initialise_event_info_struct", message);
 	}
 fail:
 	report_error("initialise_event_info_struct: initialise failed!\n");
@@ -4365,10 +4373,23 @@ JNIEXPORT void JNICALL RXTXPort(eventLoop)( JNIEnv *env, jobject jobj )
 	eis.initialised = 0;
 
 	ENTER( "eventLoop\n" );
+
+	/* If initialisation fails, we throw a Java exception, so the
+	 * Java main thread has a chance to learn what went wrong.
+	 * In that case, the monitor thread must notify the main thread
+	 * and release the lock to avoid blocking the main thread
+	 * indefinitely.
+	 * We must also defer the notify/unlock sequence at least until
+	 * the info that something went wrong has been made available
+	 * to the main thread (via the `pendingException` field), so that
+	 * when the main thread returns from `await`, it gets a chance
+	 * to know that it has to bail and not enter its main loop.
+	 * Due to that constraint, let the Java catch block be responsible
+	 * for the notify/unlock dance if initialisation fails. */
 	if ( !initialise_event_info_struct( &eis ) ) goto end;
 	if ( !init_threads( &eis ) ) goto end;
 
-	if (signal_monitor_thread_ready(env, jobj)) goto end;
+	if (signal_monitor_thread_init_completed(env, jobj)) goto end;
 	if (unlock_monitor_thread(env, jobj)) goto end;
 
 	do{

diff --git a/src/main/java/gnu/io/MonitorInitializationException.java b/src/main/java/gnu/io/MonitorInitializationException.java
@@ -0,0 +1,78 @@
+/*-------------------------------------------------------------------------
+|   RXTX License v 2.1 - LGPL v 2.1 + Linking Over Controlled Interface.
+|   RXTX is a native interface to serial ports in java.
+|   Copyright 1997-2025 by Trent Jarvi [email protected] and others who
+|   actually wrote it.  See individual source files for more information.
+|
+|   A copy of the LGPL v 2.1 may be found at
+|   http://www.gnu.org/licenses/lgpl.txt on March 4th 2007.  A copy is
+|   here for your convenience.
+|
+|   This library is free software; you can redistribute it and/or
+|   modify it under the terms of the GNU Lesser General Public
+|   License as published by the Free Software Foundation; either
+|   version 2.1 of the License, or (at your option) any later version.
+|
+|   This library is distributed in the hope that it will be useful,
+|   but WITHOUT ANY WARRANTY; without even the implied warranty of
+|   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+|   Lesser General Public License for more details.
+|
+|   An executable that contains no derivative of any portion of RXTX, but
+|   is designed to work with RXTX by being dynamically linked with it,
+|   is considered a "work that uses the Library" subject to the terms and
+|   conditions of the GNU Lesser General Public License.
+|
+|   The following has been added to the RXTX License to remove
+|   any confusion about linking to RXTX.   We want to allow in part what
+|   section 5, paragraph 2 of the LGPL does not permit in the special
+|   case of linking over a controlled interface.  The intent is to add a
+|   Java Specification Request or standards body defined interface in the 
+|   future as another exception but one is not currently available.
+|
+|   http://www.fsf.org/licenses/gpl-faq.html#LinkingOverControlledInterface
+|
+|   As a special exception, the copyright holders of RXTX give you
+|   permission to link RXTX with independent modules that communicate with
+|   RXTX solely through the Sun Microsytems CommAPI interface version 2,
+|   regardless of the license terms of these independent modules, and to copy
+|   and distribute the resulting combined work under terms of your choice,
+|   provided that every copy of the combined work is accompanied by a complete
+|   copy of the source code of RXTX (the version of RXTX used to produce the
+|   combined work), being distributed under the terms of the GNU Lesser General
+|   Public License plus this exception.  An independent module is a
+|   module which is not derived from or based on RXTX.
+|
+|   Note that people who make modified versions of RXTX are not obligated
+|   to grant this special exception for their modified versions; it is
+|   their choice whether to do so.  The GNU Lesser General Public License
+|   gives permission to release a modified version without this exception; this
+|   exception also makes it possible to release a modified version which
+|   carries forward this exception.
+|
+|   You should have received a copy of the GNU Lesser General Public
+|   License along with this library; if not, write to the Free
+|   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+|   All trademarks belong to their respective owners.
+--------------------------------------------------------------------------*/
+package gnu.io;
+
+/**
+* The monitor thread encountered an error while initializing the
+* event loop.
+*/
+@SuppressWarnings("serial")
+public class MonitorInitializationException extends RuntimeException
+{
+	public MonitorInitializationException(String str)
+	{
+		super(str);
+	}
+	public MonitorInitializationException()
+	{
+		super();
+	}
+	public MonitorInitializationException(String message, Throwable cause) {
+		super(message, cause);
+	}
+}
diff --git a/src/main/java/gnu/io/RXTXPort.java b/src/main/java/gnu/io/RXTXPort.java
@@ -62,6 +62,7 @@
 import java.util.TooManyListenersException;
 import java.lang.Math;
 import java.util.concurrent.*;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.concurrent.locks.Condition;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
@@ -94,6 +95,12 @@ class MonitorThread extends Thread
 		private volatile boolean Data=false;
 		private volatile boolean Output=false;
 
+		/**
+		 * Whether an exception occurred during initialization.
+		 * The controlling thread may read and propagate it.
+		 */
+		private final AtomicReference<MonitorInitializationException> pendingException = new AtomicReference<>();
+
 		MonitorThread() 
 		{
 			if (debug)
@@ -112,7 +119,16 @@ public void run()
 	            eis = 0;
 				if (debug)
 					z.reportln( "eventLoop() returned, this is invalid."); 
-			}catch(Throwable ex) {
+			} catch (MonitorInitializationException ex) {
+				if (debug) {
+					z.reportln("Error while initializing monitor thread; propagating.");
+				}
+				this.pendingException.set(ex);
+				if (monitorThreadState.isWriteLockedByCurrentThread()) {
+					monitorThreadInitCompleted.signal();
+					monitorThreadState.writeLock().unlock();
+				}
+			} catch (Throwable ex) {
 				HARDWARE_FAULT=true;
 				sendEvent(SerialPortEvent.HARDWARE_ERROR, true);
 			}
@@ -122,6 +138,14 @@ protected void finalize() throws Throwable
 			if (debug)
 				z.reportln( "RXTXPort:MonitorThread exiting"); 
 		}
+
+		/**
+		 * Atomically clears and returns a pending exception if initialization has failed.
+		 * @return an exception if one is pending, <code>null</code> otherwise.
+		 */
+		public MonitorInitializationException popPendingException() {
+			return pendingException.getAndSet(null);
+		}
 	}
 	protected boolean HARDWARE_FAULT=false;
 	protected final static boolean debug = false;
@@ -180,10 +204,14 @@ public RXTXPort( String name ) throws PortInUseException
 			monThread.setName("RXTXPortMonitor("+name+")");
 			monThread.start();
 			try {
-				this.monitorThreadReady.await();
+				this.monitorThreadInitCompleted.await();
 			} catch (InterruptedException e) {
 				z.reportln("Interrupted while waiting for the monitor thread to start!");
 			}
+			MonitorInitializationException exception = monThread.popPendingException();
+			if (exception != null) {
+				throw exception;
+			}
 			this.monitorThreadState.writeLock().unlock();
 	//	} catch ( PortInUseException e ){}
 		timeout = -1;	/* default disabled timeout */
@@ -692,11 +720,16 @@ protected native int readTerminatedArray( byte b[], int off, int len, byte t[] )
 	private final Lock monitorThreadStateWriteLock = this.monitorThreadState.writeLock();
 
 	/**
-	 * Signalled by JNI code from inside {@link #eventLoop()} when the event
-	 * loop has finished initialization of the native data structures and is
-	 * ready to service events.
+	 * <p>Signalled by JNI code from inside {@link #eventLoop()} when the event
+	 * loop has finished initialization of the native data structures.</p>
+	 *
+	 * <p>This either means that initialization was successful (and the
+	 * event loop is ready to service events) or an exception occurred
+	 * during initialization.
+	 * In the latter case, {@link MonitorThread#pendingException} contains
+	 * a non-null value.</p>
 	 */
-	private final Condition monitorThreadReady = this.monitorThreadState.writeLock().newCondition();
+	private final Condition monitorThreadInitCompleted = this.monitorThreadState.writeLock().newCondition();
 
 	/** Process SerialPortEvents */
 	native void eventLoop();
@@ -911,10 +944,14 @@ public void addEventListener(
 			monThread.setName("RXTXPortMonitor("+name+")");
 			monThread.start();
 			try {
-				this.monitorThreadReady.await();
+				this.monitorThreadInitCompleted.await();
 			} catch (InterruptedException e) {
 				z.reportln("Interrupted while waiting for the monitor thread to start!");
 			}
+			MonitorInitializationException exception = monThread.popPendingException();
+			if (exception != null) {
+				throw exception;
+			}
 			this.monitorThreadState.writeLock().unlock();
 		}
 		if (debug)

diff --git a/src/main/resources/com/neuronrobotics/nrjavaserial/build.properties b/src/main/resources/com/neuronrobotics/nrjavaserial/build.properties
@@ -1,2 +1,2 @@
 app.name = nrjavaserial
-app.version = 5.2.1+dbsystel2
+app.version = 5.2.1+dbsystel3