Unreachable native calls 107

pom.xml

@@ -88,6 +88,18 @@
       <version>${dependency.junit.version}</version>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.powermock</groupId>
+      <artifactId>powermock-module-junit4</artifactId>
+      <version>2.0.9</version>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.powermock</groupId>
+      <artifactId>powermock-api-mockito2</artifactId>
+      <version>2.0.9</version>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>ch.qos.logback</groupId>
       <artifactId>logback-classic</artifactId>

src/main/java/jssc/SerialPort.java

@@ -25,7 +25,6 @@
 package jssc;
 
 import java.io.UnsupportedEncodingException;
-import java.lang.reflect.Method;
 
 /**
  *
@@ -42,10 +41,6 @@ public class SerialPort {
     private boolean maskAssigned = false;
     private boolean eventListenerAdded = false;
 
-    //since 2.2.0 ->
-    private volatile Method methodErrorOccurred = null;
-    //<- since 2.2.0
-
     /** Baud rate 110 symbols/second **/
     public static final int BAUDRATE_110 = 110;
     /** Baud rate 300 symbols/second **/
@@ -1094,19 +1089,6 @@ private synchronized void addEventListener(SerialPortEventListener listener, int
             eventListener = listener;
             eventThread = getNewEventThread();
             eventThread.setName("EventThread " + portName);
-            //since 2.2.0 ->
-            try {
-                Method method = eventListener.getClass().getMethod("errorOccurred", new Class<?>[]{SerialPortException.class});
-                method.setAccessible(true);
-                methodErrorOccurred = method;
-            }
-            catch (SecurityException ex) {
-                //Do nothing
-            }
-            catch (NoSuchMethodException ex) {
-                //Do nothing
-            }
-            //<- since 2.2.0
             eventThread.start();
             eventListenerAdded = true;
         }
@@ -1154,7 +1136,6 @@ public synchronized boolean removeEventListener() throws SerialPortException {
                 }
             }
         }
-        methodErrorOccurred = null;
         eventListenerAdded = false;
         return true;
     }
@@ -1192,15 +1173,6 @@ public void run() {
                 for(int[] event : eventArray){
                     if(event[0] > 0 && !threadTerminated){
                         eventListener.serialEvent(new SerialPortEvent(SerialPort.this, event[0], event[1]));
-                        //FIXME
-                        /*if(methodErrorOccurred != null){
-                            try {
-                                methodErrorOccurred.invoke(eventListener, new Object[]{new SerialPortException(SerialPort.this, "method", "exception")});
-                            }
-                            catch (Exception ex) {
-                                System.out.println(ex);
-                            }
-                        }*/
                     }
                 }
             }

src/test/java/jssc/NativeMethodInvocationTest.java

@@ -0,0 +1,188 @@
+package jssc;
+
+import junit.framework.TestCase;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+import org.powermock.reflect.Whitebox;
+
+import static org.mockito.Mockito.*;
+
+/**
+ * Tests if Java logic around native invocations does not prevent
+ * critical calls from happening when opening and closing ports
+ */
+@RunWith(PowerMockRunner.class)
+@PrepareForTest(fullyQualifiedNames = "jssc.*")
+public class NativeMethodInvocationTest extends TestCase {
+
+    @Mock(name = "serialInterface")
+    private SerialNativeInterface serialInterface;
+
+    private final long mockHandle = 0xDeadDeefL;
+
+    @Test
+    public void nativeMethodIsNotCalledWhenAttemptingToOpenAlreadyOpenPort() {
+        // given
+        SerialPort serialPort = newSerialPort();
+
+        // when
+        try{
+            serialPort.openPort();
+            serialPort.openPort();
+            Assert.fail("Expected to throw a SerialPortException");
+        } catch (SerialPortException expected) {
+            // TODO: Is it really expected or should this method return false as javadoc states?
+        }
+
+        // then
+        verify(serialInterface, times(1)).openPort(anyString(), anyBoolean());
+    }
+
+    @Test
+    public void nativeMethodIsNotCalledWhenAttemptingToClosePortThatIsNotYetOpen() {
+        // given
+        SerialPort serialPort = newSerialPort();
+
+        // when
+        try{
+            serialPort.closePort();
+            Assert.fail("Expected to throw a SerialPortException");
+        } catch (SerialPortException expected) {
+            // TODO: Is it really expected or should this method return false as javadoc states?
+        }
+
+        // then
+        verify(serialInterface, never()).setEventsMask(anyLong(), anyInt());
+        verify(serialInterface, never()).closePort(anyLong());
+    }
+
+    @Test
+    public void nativeMethodIsNotCalledWhenAttemptingCloseAlreadyClosedPort() {
+        // given
+        SerialPort serialPort = newSerialPort();
+
+        // when
+        try{
+            serialPort.openPort();
+            serialPort.closePort();
+            serialPort.closePort();
+            Assert.fail("Expected to throw a SerialPortException");
+        } catch (SerialPortException expected) {
+            // TODO: Is it really expected or should this method return false as javadoc states?
+        }
+
+        // then
+        verify(serialInterface, times(1)).openPort(anyString(), anyBoolean());
+        verify(serialInterface, times(1)).closePort(anyLong());
+    }
+
+    @Test
+    public void nativeMethodIsCalledWhenClosingOpenAndReopeningClosedPort() throws SerialPortException {
+        // given
+        SerialPort serialPort = newSerialPort();
+
+        // when
+        serialPort.openPort();
+        serialPort.closePort();
+        serialPort.openPort();
+        serialPort.closePort();
+
+        // then
+        verify(serialInterface, times(2)).openPort(anyString(), anyBoolean());
+        verify(serialInterface, times(2)).closePort(mockHandle);
+    }
+
+    @Test
+    public void nativeMethodIsCalledWhenClosingOpenPortWithEventListenerOnPosix() throws SerialPortException {
+        // given
+        setOs(SerialNativeInterface.OS_LINUX);
+        SerialPort serialPort = newSerialPort();
+        when(serialInterface.setEventsMask(anyLong(), anyInt())).thenReturn(true);
+
+        // when
+        serialPort.openPort();
+        serialPort.addEventListener(someListener(), SerialPort.MASK_RXCHAR);
+        serialPort.closePort();
+
+        // then
+        verify(serialInterface, times(1)).openPort(anyString(), anyBoolean());
+        verify(serialInterface, never()).setEventsMask(anyLong(), anyInt());
+        verify(serialInterface, times(1)).closePort(mockHandle);
+    }
+
+    @Test
+    public void nativeMethodIsCalledWhenClosingOpenPortWithEventListenerOnWindows() throws SerialPortException {
+        // given
+        setOs(SerialNativeInterface.OS_WINDOWS);
+        SerialPort serialPort = newSerialPort();
+        when(serialInterface.setEventsMask(anyLong(), anyInt())).thenReturn(true);
+
+        // when
+        serialPort.openPort();
+        serialPort.addEventListener(someListener(), SerialPort.MASK_RXCHAR);
+        serialPort.closePort();
+
+        // then
+        verify(serialInterface, times(1)).openPort(anyString(), anyBoolean());
+        verify(serialInterface, times(1)).setEventsMask(mockHandle, SerialPort.MASK_RXCHAR);
+        verify(serialInterface, times(1)).setEventsMask(mockHandle, 0);
+        verify(serialInterface, times(1)).closePort(mockHandle);
+    }
+
+    /**
+     * Simulates conditions as described in issue #107: user attempts to close the serial port that has been removed
+     * from the system (e.g. unplugged usb serial adapter) without notifying the java code about it.
+     */
+    @Test
+    public void nativeMethodIsCalledWhenClosingOpenPortWithEventListenerIfSetMaskFails() {
+        // given
+        setOs(SerialNativeInterface.OS_WINDOWS);
+        SerialPort serialPort = newSerialPort();
+        when(serialInterface.setEventsMask(anyLong(), anyInt())).thenReturn(true);
+
+        // when
+        try {
+            serialPort.openPort();
+            serialPort.addEventListener(someListener());
+            when(serialInterface.setEventsMask(anyLong(), anyInt())).thenReturn(false);
+            serialPort.closePort();
+        } catch (SerialPortException expected) {
+            // TODO: Is it really expected or should this method return false as javadoc states?
+        }
+
+        // then
+        verify(serialInterface, times(1)).openPort(anyString(), anyBoolean());
+        verify(serialInterface, times(1)).setEventsMask(mockHandle, SerialPort.MASK_RXCHAR);
+        verify(serialInterface, times(1)).setEventsMask(mockHandle, 0);
+        verify(serialInterface, times(1)).closePort(mockHandle);
+    }
+
+    private void setOs(int os) {
+        PowerMockito.mockStatic(SerialNativeInterface.class);
+        when(SerialNativeInterface.getOsType()).thenReturn(os);
+    }
+
+    private SerialPort newSerialPort() {
+        String portName = "dummy";
+        SerialPort serialPort = new SerialPort(portName);
+        Whitebox.setInternalState(serialPort, "serialInterface", serialInterface);
+        when(serialInterface.openPort(anyString(), anyBoolean())).thenReturn(mockHandle);
+        when(serialInterface.closePort(anyLong())).thenReturn(true);
+        when(serialInterface.waitEvents(anyLong())).thenReturn(new int[][]{});
+        return serialPort;
+    }
+
+    private SerialPortEventListener someListener() {
+        return new SerialPortEventListener() {
+            @Override
+            public void serialEvent(SerialPortEvent serialPortEvent) {
+
+            }
+        };
+    }
+}