Better handling of closed stream errors (#1304)

Add catches to explicitly close streams if a ClosedChannelException is thrown Add better lifecycle management for streamable interfaces
qzind · Dec 3, 2024 · b1573f8 · b1573f8
1 parent 6fa6ce6
commit b1573f8
Show file tree

Hide file tree

Showing 17 changed files with 170 additions and 78 deletions.
diff --git a/src/qz/communication/DeviceIO.java b/src/qz/communication/DeviceIO.java
@@ -1,6 +1,6 @@
 package qz.communication;
 
-public interface DeviceIO {
+public interface DeviceIO extends DeviceListener {
 
     String getVendorId();
 
@@ -11,8 +11,7 @@ public interface DeviceIO {
 
     boolean isOpen();
 
-    void close() throws DeviceException;
-
+    void close();
 
     void setStreaming(boolean streaming);
 

diff --git a/src/qz/communication/DeviceListener.java b/src/qz/communication/DeviceListener.java
@@ -1,7 +1,9 @@
 package qz.communication;
 
 public interface DeviceListener {
-
+    /**
+     * Cleanup task for when a socket closes while a device is still streaming
+     */
     void close();
 
 }
diff --git a/src/qz/communication/FileIO.java b/src/qz/communication/FileIO.java
@@ -9,11 +9,12 @@
 import qz.ws.PrintSocketClient;
 import qz.ws.StreamEvent;
 
+import java.nio.channels.ClosedChannelException;
 import java.nio.file.Path;
 import java.nio.file.WatchKey;
 import java.util.ArrayList;
 
-public class FileIO {
+public class FileIO implements DeviceListener {
     public static final String SANDBOX_DATA_SUFFIX = "sandbox";
     public static final String GLOBAL_DATA_SUFFIX = "shared";
     public static final int FILE_LISTENER_DEFAULT_LINES = 10;
@@ -133,7 +134,7 @@ public void setWk(WatchKey wk) {
         this.wk = wk;
     }
 
-    public void fileChanged(String fileName, String type, String fileData) {
+    public void fileChanged(String fileName, String type, String fileData) throws ClosedChannelException {
         StreamEvent evt = new StreamEvent(StreamEvent.Stream.FILE, StreamEvent.Type.ACTION)
                 .withData("file", getOriginalPath().resolve(fileName))
                 .withData("eventType", type);
@@ -145,13 +146,14 @@ public void fileChanged(String fileName, String type, String fileData) {
         PrintSocketClient.sendStream(session, evt);
     }
 
-    public void sendError(String message) {
+    public void sendError(String message) throws ClosedChannelException {
         StreamEvent eventErr = new StreamEvent(StreamEvent.Stream.FILE, StreamEvent.Type.ERROR)
                 .withData("message", message);
         PrintSocketClient.sendStream(session, eventErr);
     }
 
 
+    @Override
     public void close() {
         if (wk != null) {
             wk.cancel();

diff --git a/src/qz/communication/H4J_HidIO.java b/src/qz/communication/H4J_HidIO.java
@@ -1,21 +1,27 @@
 package qz.communication;
 
 import org.hid4java.HidDevice;
+import qz.ws.SocketConnection;
 
 import javax.usb.util.UsbUtil;
 
-public class H4J_HidIO implements DeviceIO {
+public class H4J_HidIO implements DeviceIO, DeviceListener {
 
     private HidDevice device;
 
     private boolean streaming;
 
+    private DeviceOptions dOpts;
+    private SocketConnection websocket;
 
-    public H4J_HidIO(DeviceOptions dOpts) throws DeviceException {
-        this(H4J_HidUtilities.findDevice(dOpts));
+
+    public H4J_HidIO(DeviceOptions dOpts, SocketConnection websocket) throws DeviceException {
+        this(H4J_HidUtilities.findDevice(dOpts), dOpts, websocket);
     }
 
-    public H4J_HidIO(HidDevice device) throws DeviceException {
+    private H4J_HidIO(HidDevice device, DeviceOptions dOpts, SocketConnection websocket) throws DeviceException {
+        this.dOpts = dOpts;
+        this.websocket = websocket;
         if (device == null) {
             throw new DeviceException("HID device could not be found");
         }
@@ -30,7 +36,7 @@ public void open() {
     }
 
     public boolean isOpen() {
-        return device.isOpen();
+        return !device.isClosed();
     }
 
     public void setStreaming(boolean active) {
@@ -91,11 +97,14 @@ public void sendFeatureReport(byte[] data, Byte reportId) throws DeviceException
         }
     }
 
+    @Override
     public void close() {
+        setStreaming(false);
+        // Remove orphaned reference
+        websocket.removeDevice(dOpts);
         if (isOpen()) {
             device.close();
         }
-        streaming = false;
     }
 
 }
diff --git a/src/qz/communication/H4J_HidListener.java b/src/qz/communication/H4J_HidListener.java
@@ -30,7 +30,7 @@ public H4J_HidListener(Session session) {
     @Override
     public void hidFailure(HidServicesEvent hidServicesEvent) {
         log.debug("Device failure: {}", hidServicesEvent.getHidDevice().getProduct());
-        PrintSocketClient.sendStream(session, createStreamAction(hidServicesEvent.getHidDevice(), "Device Failure"));
+        PrintSocketClient.sendStream(session, createStreamAction(hidServicesEvent.getHidDevice(), "Device Failure"), this);
     }
 
     @Override
@@ -42,19 +42,19 @@ public void hidDataReceived(HidServicesEvent hidServicesEvent) {
             hex.put(UsbUtil.toHexString(b));
         }
 
-        PrintSocketClient.sendStream(session, createStreamAction(hidServicesEvent.getHidDevice(), "Data Received", hex));
+        PrintSocketClient.sendStream(session, createStreamAction(hidServicesEvent.getHidDevice(), "Data Received", hex), this);
     }
 
     @Override
     public void hidDeviceDetached(HidServicesEvent hidServicesEvent) {
         log.debug("Device detached: {}", hidServicesEvent.getHidDevice().getProduct());
-        PrintSocketClient.sendStream(session, createStreamAction(hidServicesEvent.getHidDevice(), "Device Detached"));
+        PrintSocketClient.sendStream(session, createStreamAction(hidServicesEvent.getHidDevice(), "Device Detached"), this);
     }
 
     @Override
     public void hidDeviceAttached(HidServicesEvent hidServicesEvent) {
         log.debug("Device attached: {}", hidServicesEvent.getHidDevice().getProduct());
-        PrintSocketClient.sendStream(session, createStreamAction(hidServicesEvent.getHidDevice(), "Device Attached"));
+        PrintSocketClient.sendStream(session, createStreamAction(hidServicesEvent.getHidDevice(), "Device Attached"), this);
     }
 
     private StreamEvent createStreamAction(HidDevice device, String action) {

diff --git a/src/qz/communication/PJHA_HidIO.java b/src/qz/communication/PJHA_HidIO.java
@@ -7,6 +7,7 @@
 import purejavahidapi.InputReportListener;
 import purejavahidapi.PureJavaHidApi;
 import qz.utils.SystemUtilities;
+import qz.ws.SocketConnection;
 
 import javax.usb.util.UsbUtil;
 import java.io.IOException;
@@ -22,13 +23,16 @@ public class PJHA_HidIO implements DeviceIO {
     private static final int BUFFER_SIZE = 32;
     private Vector<byte[]> dataBuffer;
     private boolean streaming;
+    private DeviceOptions dOpts;
+    private SocketConnection websocket;
 
-
-    public PJHA_HidIO(DeviceOptions dOpts) throws DeviceException {
-        this(PJHA_HidUtilities.findDevice(dOpts));
+    public PJHA_HidIO(DeviceOptions dOpts, SocketConnection websocket) throws DeviceException {
+        this(PJHA_HidUtilities.findDevice(dOpts), dOpts, websocket);
     }
 
-    public PJHA_HidIO(HidDeviceInfo deviceInfo) throws DeviceException {
+    private PJHA_HidIO(HidDeviceInfo deviceInfo, DeviceOptions dOpts, SocketConnection websocket) throws DeviceException {
+        this.dOpts = dOpts;
+        this.websocket = websocket;
         if (deviceInfo == null) {
             throw new DeviceException("HID device could not be found");
         }
@@ -129,7 +133,11 @@ public void sendFeatureReport(byte[] data, Byte reportId) throws DeviceException
 
     }
 
+    @Override
     public void close() {
+        setStreaming(false);
+        // Remove orphaned reference
+        websocket.removeDevice(dOpts);
         if (isOpen()) {
             try {
                 device.setInputReportListener(null);
@@ -140,7 +148,6 @@ public void close() {
             }
         }
 
-        streaming = false;
         device = null;
     }
 

diff --git a/src/qz/communication/PJHA_HidListener.java b/src/qz/communication/PJHA_HidListener.java
@@ -45,6 +45,6 @@ public void close() {
     @Override
     public void onDeviceRemoval(HidDevice device) {
         log.debug("Device detached: {}", device.getHidDeviceInfo().getProductString());
-        PrintSocketClient.sendStream(session, createStreamAction(device, "Device Detached"));
+        PrintSocketClient.sendStream(session, createStreamAction(device, "Device Detached"), this);
     }
 }
diff --git a/src/qz/communication/SerialIO.java b/src/qz/communication/SerialIO.java
@@ -9,13 +9,14 @@
 import qz.common.ByteArrayBuilder;
 import qz.utils.ByteUtilities;
 import qz.utils.DeviceUtilities;
+import qz.ws.SocketConnection;
 
 import java.io.IOException;
 
 /**
  * @author Tres
  */
-public class SerialIO {
+public class SerialIO implements DeviceListener {
 
     private static final Logger log = LogManager.getLogger(SerialIO.class);
 
@@ -28,14 +29,17 @@ public class SerialIO {
 
     private ByteArrayBuilder data = new ByteArrayBuilder();
 
+    private SocketConnection websocket;
+
 
     /**
      * Controller for serial communications
      *
      * @param portName Port name to open, such as "COM1" or "/dev/tty0/"
      */
-    public SerialIO(String portName) {
+    public SerialIO(String portName, SocketConnection websocket) {
         this.portName = portName;
+        this.websocket = websocket;
     }
 
     /**
@@ -257,26 +261,31 @@ public void sendData(JSONObject params, SerialOptions opts) throws JSONException
     /**
      * Closes the serial port, if open.
      *
-     * @return Boolean indicating success.
      * @throws SerialPortException If the port fails to close.
      */
-    public boolean close() throws SerialPortException {
+    @Override
+    public void close() {
+        // Remove orphaned reference
+        websocket.removeSerialPort(portName);
+
         if (!isOpen()) {
             log.warn("Serial port [{}] is not open.", portName);
-            return false;
         }
 
-        boolean closed = port.closePort();
-        if (closed) {
-            log.info("Serial port [{}] closed successfully.", portName);
-        } else {
+        try {
+            boolean closed = port.closePort();
+            if (closed) {
+                log.info("Serial port [{}] closed successfully.", portName);
+            } else {
+                // Handle ambiguity in JSSCs API
+                throw new SerialPortException(portName, "closePort", "Port not closed");
+            }
+        } catch(SerialPortException e) {
             log.warn("Serial port [{}] was not closed properly.", portName);
         }
 
         port = null;
         portName = null;
-
-        return closed;
     }
 
     private Integer min(Integer a, Integer b) {

diff --git a/src/qz/communication/SocketIO.java b/src/qz/communication/SocketIO.java
@@ -7,6 +7,7 @@
 import org.apache.logging.log4j.Logger;
 import qz.utils.DeviceUtilities;
 import qz.utils.NetworkUtilities;
+import qz.ws.SocketConnection;
 
 import java.io.DataInputStream;
 import java.io.DataOutputStream;
@@ -15,7 +16,7 @@
 import java.nio.charset.Charset;
 import java.util.ArrayList;
 
-public class SocketIO {
+public class SocketIO implements DeviceListener {
 
     private static final Logger log = LogManager.getLogger(SocketIO.class);
 
@@ -27,10 +28,13 @@ public class SocketIO {
     private DataOutputStream dataOut;
     private DataInputStream dataIn;
 
-    public SocketIO(String host, int port, Charset encoding) {
+    private SocketConnection websocket;
+
+    public SocketIO(String host, int port, Charset encoding, SocketConnection websocket) {
         this.host = host;
         this.port = port;
         this.encoding = encoding;
+        this.websocket = websocket;
     }
 
     public boolean open() throws IOException {
@@ -68,9 +72,21 @@ public String processSocketResponse() throws IOException {
         return null;
     }
 
-    public void close() throws IOException {
-        dataOut.close();
-        socket.close();
+    @Override
+    public void close() {
+        // Remove orphaned reference
+        websocket.removeNetworkSocket(String.format("%s:%s", host, port));
+
+        try {
+            dataOut.close();
+        } catch(IOException e) {
+            log.warn("Could not close socket output stream", e);
+        }
+        try {
+            socket.close();
+        } catch(IOException e) {
+            log.warn("Could not close socket", e);
+        }
     }
 
     public String getHost() {