From 8b9ad7efdf3cf9008472450c41483540b45c6f60 Mon Sep 17 00:00:00 2001 From: Kai Morich Date: Wed, 24 Apr 2024 21:24:24 +0200 Subject: [PATCH] improved error handling for read() with concurrent close() (#569) - isOpen() returns false during concurrent close() - less tracing in SerialInputOutputManager --- usbSerialForAndroid/build.gradle | 2 +- .../hoho/android/usbserial/DeviceTest.java | 3 +++ .../usbserial/driver/CommonUsbSerialPort.java | 22 +++++++++++-------- .../usbserial/driver/FtdiSerialDriver.java | 4 ++-- .../driver/ProlificSerialDriver.java | 4 ++-- .../util/SerialInputOutputManager.java | 6 ++++- 6 files changed, 26 insertions(+), 15 deletions(-) diff --git a/usbSerialForAndroid/build.gradle b/usbSerialForAndroid/build.gradle index ee91de16..aa7f699f 100644 --- a/usbSerialForAndroid/build.gradle +++ b/usbSerialForAndroid/build.gradle @@ -13,7 +13,7 @@ android { testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" testInstrumentationRunnerArguments = [ // Raspi Windows LinuxVM ... - 'rfc2217_server_host': '192.168.0.147', + 'rfc2217_server_host': '192.168.0.78', 'rfc2217_server_nonstandard_baudrates': 'true', // true false false ] } diff --git a/usbSerialForAndroid/src/androidTest/java/com/hoho/android/usbserial/DeviceTest.java b/usbSerialForAndroid/src/androidTest/java/com/hoho/android/usbserial/DeviceTest.java index 617afde3..ca50ec7e 100644 --- a/usbSerialForAndroid/src/androidTest/java/com/hoho/android/usbserial/DeviceTest.java +++ b/usbSerialForAndroid/src/androidTest/java/com/hoho/android/usbserial/DeviceTest.java @@ -320,8 +320,10 @@ public void run() { try { closer.wait = false; usb.serialPort.read(new byte[256], 2000); + fail("closed expected"); } catch(IOException ex) { + assertFalse(usb.serialPort.isOpen()); assertEquals("Connection closed", ex.getMessage()); } th.join(); @@ -333,6 +335,7 @@ public void run() { usb.serialPort.read(new byte[256], 0); fail("closed expected"); } catch(IOException ex) { + assertFalse(usb.serialPort.isOpen()); assertEquals("Connection closed", ex.getMessage()); } th.join(); diff --git a/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/CommonUsbSerialPort.java b/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/CommonUsbSerialPort.java index 8cb460e9..99a912de 100644 --- a/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/CommonUsbSerialPort.java +++ b/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/CommonUsbSerialPort.java @@ -139,6 +139,8 @@ public void close() throws IOException { if (mConnection == null) { throw new IOException("Already closed"); } + UsbDeviceConnection connection = mConnection; + mConnection = null; try { mUsbRequest.cancel(); } catch(Exception ignored) {} @@ -147,9 +149,8 @@ public void close() throws IOException { closeInt(); } catch(Exception ignored) {} try { - mConnection.close(); + connection.close(); } catch(Exception ignored) {} - mConnection = null; } protected abstract void closeInt(); @@ -157,10 +158,13 @@ public void close() throws IOException { /** * use simple USB request supported by all devices to test if connection is still valid */ - protected void testConnection() throws IOException { - if(mConnection == null || mUsbRequest == null) { + protected void testConnection(boolean full) throws IOException { + if(mConnection == null) { throw new IOException("Connection closed"); } + if(!full) { + return; + } byte[] buf = new byte[2]; int len = mConnection.controlTransfer(0x80 /*DEVICE*/, 0 /*GET_STATUS*/, 0, 0, buf, buf.length, 200); if(len < 0) @@ -169,7 +173,7 @@ protected void testConnection() throws IOException { @Override public int read(final byte[] dest, final int timeout) throws IOException { - if(dest.length <= 0) { + if(dest.length == 0) { throw new IllegalArgumentException("Read buffer too small"); } return read(dest, dest.length, timeout); @@ -179,7 +183,7 @@ public int read(final byte[] dest, final int timeout) throws IOException { public int read(final byte[] dest, final int length, final int timeout) throws IOException {return read(dest, length, timeout, true);} protected int read(final byte[] dest, int length, final int timeout, boolean testConnection) throws IOException { - if(mConnection == null || mUsbRequest == null) { + if(mConnection == null) { throw new IOException("Connection closed"); } if(length <= 0) { @@ -201,8 +205,8 @@ protected int read(final byte[] dest, int length, final int timeout, boolean tes nread = mConnection.bulkTransfer(mReadEndpoint, dest, readMax, timeout); // Android error propagation is improvable: // nread == -1 can be: timeout, connection lost, buffer to small, ??? - if(nread == -1 && testConnection && MonotonicClock.millis() < endTime) - testConnection(); + if(nread == -1 && testConnection) + testConnection(MonotonicClock.millis() < endTime); } else { final ByteBuffer buf = ByteBuffer.wrap(dest, 0, length); @@ -217,7 +221,7 @@ protected int read(final byte[] dest, int length, final int timeout, boolean tes // Android error propagation is improvable: // response != null & nread == 0 can be: connection lost, buffer to small, ??? if(nread == 0) { - testConnection(); + testConnection(true); } } return Math.max(nread, 0); diff --git a/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/FtdiSerialDriver.java b/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/FtdiSerialDriver.java index 8f5f88f7..5e3c43b2 100644 --- a/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/FtdiSerialDriver.java +++ b/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/FtdiSerialDriver.java @@ -165,8 +165,8 @@ public int read(final byte[] dest, int length, final int timeout) throws IOExcep do { nread = super.read(dest, length, Math.max(1, (int)(endTime - MonotonicClock.millis())), false); } while (nread == READ_HEADER_LENGTH && MonotonicClock.millis() < endTime); - if(nread <= 0 && MonotonicClock.millis() < endTime) - testConnection(); + if(nread <= 0) + testConnection(MonotonicClock.millis() < endTime); } else { do { nread = super.read(dest, length, timeout); diff --git a/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/ProlificSerialDriver.java b/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/ProlificSerialDriver.java index 92c8a65e..b8257cb8 100644 --- a/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/ProlificSerialDriver.java +++ b/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/ProlificSerialDriver.java @@ -205,8 +205,8 @@ private void readStatusThreadFunction() { byte[] buffer = new byte[STATUS_BUFFER_SIZE]; long endTime = MonotonicClock.millis() + 500; int readBytesCount = mConnection.bulkTransfer(mInterruptEndpoint, buffer, STATUS_BUFFER_SIZE, 500); - if(readBytesCount == -1 && MonotonicClock.millis() < endTime) - testConnection(); + if(readBytesCount == -1) + testConnection(MonotonicClock.millis() < endTime); if (readBytesCount > 0) { if (readBytesCount != STATUS_BUFFER_SIZE) { throw new IOException("Invalid status notification, expected " + STATUS_BUFFER_SIZE + " bytes, got " + readBytesCount); diff --git a/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/util/SerialInputOutputManager.java b/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/util/SerialInputOutputManager.java index 490b8d62..2d9b7b06 100644 --- a/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/util/SerialInputOutputManager.java +++ b/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/util/SerialInputOutputManager.java @@ -203,7 +203,11 @@ public void run() { step(); } } catch (Exception e) { - Log.w(TAG, "Run ending due to exception: " + e.getMessage(), e); + if(mSerialPort.isOpen()) { + Log.w(TAG, "Run ending due to exception: " + e.getMessage(), e); + } else { + Log.i(TAG, "Socket closed"); + } final Listener listener = getListener(); if (listener != null) { listener.onRunError(e);