From f638226b0cc9c2cad7b8f788c591ce9a56feb0f5 Mon Sep 17 00:00:00 2001 From: Claudia Pellegrino Date: Tue, 14 Jan 2025 13:05:48 +0100 Subject: [PATCH] Unblock main thread if monitor thread 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 and propagate it to the main thread so the caller gets to see exactly what caused the error. Co-authored-by: Jan-Niklas Dornbach Co-authored-by: Martin Mungard Co-authored-by: Richard L. Jackson --- src/main/c/include/SerialImp.h | 1 + src/main/c/src/SerialImp.c | 53 +++++++++---- .../io/MonitorInitializationException.java | 78 +++++++++++++++++++ src/main/java/gnu/io/RXTXPort.java | 57 +++++++++++--- 4 files changed, 163 insertions(+), 26 deletions(-) create mode 100644 src/main/java/gnu/io/MonitorInitializationException.java diff --git a/src/main/c/include/SerialImp.h b/src/main/c/include/SerialImp.h index 329140e0..68b54d9e 100644 --- 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/src/SerialImp.c b/src/main/c/src/SerialImp.c index aa0f048c..772331d2 100644 --- a/src/main/c/src/SerialImp.c +++ b/src/main/c/src/SerialImp.c @@ -3838,7 +3838,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. @@ -3847,35 +3848,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; } @@ -4265,6 +4266,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; @@ -4308,8 +4310,9 @@ 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){ - report_error("initialise_event_info_struct: eis->send_event == NULL!\n"); + if(eis->send_event == NULL) { + throw_java_exception(env, MONITOR_INITIALIZATION_EXCEPTION, + "initialise_event_info_struct", "eis->send_event == NULL"); goto fail; } end: @@ -4320,12 +4323,15 @@ int initialise_event_info_struct( struct event_info_struct *eis ) eis->tv_sleep.tv_usec = 100 * 1000; eis->initialised = 1; return( 1 ); - } else if(eis->fd >= FD_SETSIZE ){ - // you can reduce this limitation only with migration to epool or something like that. - report_error("initialise_event_info_struct: eis->fd >= FD_SETSIZE!\n"); - } else if(eis->fd <= 0 ){ - // you can reduce this limitation only with migration to epool or something like that. - report_error("initialise_event_info_struct: eis->fd <= 0!\n"); + } else if (eis->fd >= FD_SETSIZE) { + // you can reduce this limitation only with migration to poll or something like that. + sprintf(message, "fd == %i >= %i", eis->fd, FD_SETSIZE); + throw_java_exception(env, MONITOR_INITIALIZATION_EXCEPTION, + "initialise_event_info_struct", message); + } else if (eis->fd <= 0) { + sprintf(message, "fd == %i <= 0", eis->fd); + throw_java_exception(env, MONITOR_INITIALIZATION_EXCEPTION, + "initialise_event_info_struct", message); } fail: finalize_event_info_struct( eis ); @@ -4385,10 +4391,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 new file mode 100644 index 00000000..57c5e6e1 --- /dev/null +++ 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 tjarvi@qbang.org 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 index f0559661..4615e047 100644 --- 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 pendingException = new AtomicReference<>(); + MonitorThread() { if (debug) @@ -112,7 +119,17 @@ 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."); + } + monThreadisInterrupted=true; + 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 +139,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, null otherwise. + */ + public MonitorInitializationException popPendingException() { + return pendingException.getAndSet(null); + } } protected boolean HARDWARE_FAULT=false; protected final static boolean debug = false; @@ -180,11 +205,16 @@ 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!"); } this.monitorThreadState.writeLock().unlock(); + MonitorInitializationException exception = monThread.popPendingException(); + if (exception != null) { + close(); + throw exception; + } // } catch ( PortInUseException e ){} timeout = -1; /* default disabled timeout */ if (debug) @@ -692,11 +722,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. + *

Signalled by JNI code from inside {@link #eventLoop()} when the event + * loop has finished initialization of the native data structures.

+ * + *

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.

*/ - private final Condition monitorThreadReady = this.monitorThreadState.writeLock().newCondition(); + private final Condition monitorThreadInitCompleted = this.monitorThreadState.writeLock().newCondition(); /** Process SerialPortEvents */ native void eventLoop(); @@ -704,8 +739,8 @@ protected native int readTerminatedArray( byte b[], int off, int len, byte t[] ) /** * Whether the monitor thread has been interrupted. * - * The flag is cleared when the monitor thread is started, and set by the - * native code in {@ #interruptEventLoop()}. + * The flag is cleared when the monitor thread is started. It is set either + * by the native code in {@link #interruptEventLoop()} or if initialization fails. */ boolean monThreadisInterrupted=true; @@ -911,10 +946,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)