changeset 143:59f15c907e91

8076157: [UART] read/write must be independent operations Summary: Separate locks for read and write are introduced Reviewed-by: alkonsta
author snazarki
date Fri, 27 Mar 2015 13:31:05 +0300
parents 24c0e0ad84bc
children 6d9b5908ed85
files src/share/classes/com/oracle/dio/uart/impl/UARTImpl.java
diffstat 1 files changed, 109 insertions(+), 88 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/classes/com/oracle/dio/uart/impl/UARTImpl.java	Tue Mar 31 20:05:25 2015 +0300
+++ b/src/share/classes/com/oracle/dio/uart/impl/UARTImpl.java	Fri Mar 27 13:31:05 2015 +0300
@@ -51,6 +51,7 @@
     private boolean isWriting;
 
     private Object synchReadLock = new Object();
+    private Object synchWriteLock = new Object();
 
     private ByteBuffer writeBuffers[] = new ByteBuffer[2];
     private int writeBuffersPositions[] = new int[2];
@@ -111,22 +112,24 @@
         initPowerManagement();
     }
 
+    private class InternalRoundListener implements  InputRoundListener<UART, ByteBuffer> {
+            @Override
+            public void inputRoundCompleted(RoundCompletionEvent<UART, ByteBuffer> event) {
+                synchronized(synchReadLock){
+                    synchReadLock.notifyAll();
+                }
+            }
+
+            @Override
+            public void failed(Throwable ex, UART arg1) {
+                synchronized(synchReadLock){
+                    synchReadLock.notifyAll();
+                }
+            }
+    }
+
     private InputRoundListener getLocalInputRoundListener(){
-        return new InputRoundListener<UART, ByteBuffer>() {
-                @Override
-                public void inputRoundCompleted(RoundCompletionEvent<UART, ByteBuffer> event) {
-                    synchronized(synchReadLock){
-                        synchReadLock.notifyAll();
-                    }
-                }
-
-                @Override
-                public void failed(Throwable ex, UART arg1) {
-                    synchronized(synchReadLock){
-                        synchReadLock.notifyAll();
-                    }
-                }
-            };
+        return new InternalRoundListener();
     }
 
 
@@ -469,9 +472,8 @@
      * from that very same buffer upon invocation od the provided {@link OutputRoundListener} instance.
      */
     @Override
-    public synchronized void startWriting(ByteBuffer src, OutputRoundListener<UART, ByteBuffer> listener) throws IOException, UnavailableDeviceException, ClosedDeviceException{
+    public void startWriting(ByteBuffer src, OutputRoundListener<UART, ByteBuffer> listener) throws IOException, UnavailableDeviceException, ClosedDeviceException{
         checkPowerState();
-        checkWrite();
 
         if(src == null || listener == null){
             throw new NullPointerException(
@@ -479,13 +481,7 @@
             );
         }
 
-        writeBuffers[0] = src;
-        writeBuffersPositions[0] = src.position();
-        writeBufferIdx = 0;
-        outRoundListener = listener;
-        subscribe(UARTEvent.OUTPUT_BUFFER_EMPTY);
-        writeAsynch0(src);
-        isWriting = true;
+        writeAsync(src, null, listener);
     }
 
     /**
@@ -501,9 +497,25 @@
             );
         }
 
-        writeBuffers[1] = src2;
-        writeBuffersPositions[1] = src2.position();
-        startWriting(src1, listener);
+        writeAsync(src1, src2, listener);
+    }
+
+    private void writeAsync(ByteBuffer src1, ByteBuffer src2, OutputRoundListener<UART, ByteBuffer> listener) throws IOException,
+        UnavailableDeviceException, ClosedDeviceException {
+        synchronized(synchWriteLock) {
+            checkWrite();
+            writeBuffers[0] = src1;
+            writeBuffersPositions[0] = src1.position();
+            writeBuffers[1] = src2;
+            if (null != src2) {
+                writeBuffersPositions[1] = src2.position();
+            }
+            writeBufferIdx = 0;
+            outRoundListener = listener;
+            subscribe(UARTEvent.OUTPUT_BUFFER_EMPTY);
+            writeAsynch0(src1);
+            isWriting = true;
+        }
     }
 
     /**
@@ -512,12 +524,14 @@
     @Override
     public synchronized void stopWriting() throws IOException, UnavailableDeviceException, ClosedDeviceException{
         checkOpen();
-        if (isWriting){
-            outRoundListener = null;
-            unsubscribe(UARTEvent.OUTPUT_BUFFER_EMPTY);
-            writeBuffers[0] = writeBuffers[1] = null;
-            stopWriting0();
-            isWriting = false;
+        synchronized(synchWriteLock) {
+            if (isWriting){
+                outRoundListener = null;
+                unsubscribe(UARTEvent.OUTPUT_BUFFER_EMPTY);
+                writeBuffers[0] = writeBuffers[1] = null;
+                stopWriting0();
+                isWriting = false;
+            }
         }
     }
 
@@ -526,33 +540,22 @@
      * buffer.
      */
     @Override
-    public synchronized void startReading(ByteBuffer src, InputRoundListener<UART, ByteBuffer> listener) throws IOException, UnavailableDeviceException, ClosedDeviceException{
+    public void startReading(ByteBuffer src, InputRoundListener<UART, ByteBuffer> listener) throws IOException, UnavailableDeviceException, ClosedDeviceException{
         checkPowerState();
-        checkRead();
 
         if(src == null || listener == null){
             throw new NullPointerException(
                 ExceptionMessage.format(ExceptionMessage.UART_NULL_SRC_OR_LISTENER)
             );
         }
-
-        inRoundListener = listener;
-        readBuffers[0] = src;
-        readBuffersPositions[0] = src.position();
-        readBufferIdx = 0;
-        /*
-                        subscribe calls set_event_listener, in case of INPUT_DATA_AVAILABLE
-                        the native function checks if data available in the internal
-                        buffer and generates INPUT_DATA_AVAILABLE event if so.
-                */
-        subscribe(UARTEvent.INPUT_DATA_AVAILABLE);
+        readAsync(src, null, listener);
     }
 
     /**
      * Starts asynchronous reading in sucessive rounds.
      */
     @Override
-    public synchronized void startReading(ByteBuffer src1, ByteBuffer src2, InputRoundListener<UART, ByteBuffer> listener) throws IOException,
+    public void startReading(ByteBuffer src1, ByteBuffer src2, InputRoundListener<UART, ByteBuffer> listener) throws IOException,
             UnavailableDeviceException, ClosedDeviceException{
 
         if(src1 == null || src2 == null || listener == null){
@@ -560,22 +563,44 @@
                 ExceptionMessage.format(ExceptionMessage.UART_NULL_SRC1_OR_SRC2_OR_LISTENER)
             );
         }
-        readBuffers[1] = src2;
-        readBuffersPositions[1] = src2.position();
-        startReading(src1, listener);
+        readAsync(src1, src2, listener);
+    }
+
+    private void readAsync(ByteBuffer src1, ByteBuffer src2, InputRoundListener<UART, ByteBuffer> listener)  throws IOException,
+            UnavailableDeviceException, ClosedDeviceException {
+        synchronized(synchReadLock){
+            checkRead();
+            inRoundListener = listener;
+            readBuffers[0] = src1;
+            readBuffersPositions[0] = src1.position();
+            readBuffers[1] = src2;
+            if (null != src2) {
+                readBuffersPositions[1] = src2.position();
+            }
+            readBufferIdx = 0;
+            /*
+                subscribe calls set_event_listener, in case of INPUT_DATA_AVAILABLE
+                the native function checks if data available in the internal
+                buffer and generates INPUT_DATA_AVAILABLE event if so.
+            */
+            subscribe(UARTEvent.INPUT_DATA_AVAILABLE);
+        }
     }
 
     /**
      * Stops (cancels) the currently active reading session.
       */
     @Override
-    public synchronized void stopReading() throws IOException, UnavailableDeviceException, ClosedDeviceException{
+    public void stopReading() throws IOException, UnavailableDeviceException, ClosedDeviceException{
         checkOpen();
-        if (inRoundListener != null){
-            inRoundListener = null;
-            unsubscribe(UARTEvent.INPUT_DATA_AVAILABLE);
-            readBuffers[0] = readBuffers[1] = null;
-            stopReading0();
+        synchronized(synchReadLock) {
+            if (null != inRoundListener && !(inRoundListener instanceof InternalRoundListener) ) {
+                inRoundListener = null;
+                unsubscribe(UARTEvent.INPUT_DATA_AVAILABLE);
+                readBuffers[0] = readBuffers[1] = null;
+                // redundant function, remove later
+                stopReading0();
+            }
         }
     }
 
@@ -617,22 +642,20 @@
             UnavailableDeviceException, ClosedDeviceException{
         int ret;
 
-        synchronized(this){
-            checkRead();
+        if (dst == null){
+            throw new NullPointerException(
+                ExceptionMessage.format(ExceptionMessage.UART_NULL_DST)
+            );
         }
 
-        synchronized(handle){
+        if(!dst.hasRemaining()){
+            ret = 0;
+        }else{
+            ret = dst.position();
+            synchronized(synchReadLock){
 
-            if (dst == null){
-                throw new NullPointerException(
-                    ExceptionMessage.format(ExceptionMessage.UART_NULL_DST)
-                );
-            }
+                checkRead();
 
-            if(!dst.hasRemaining()){
-                ret = 0;
-            }else{
-                ret = dst.position();
                 /*read all data available*/
                 int readRes = read0(dst);
                 shiftBufferPosition(dst, ret + readRes);
@@ -644,7 +667,6 @@
                          * else read with timeout
                          */
                         startReading(dst, getLocalInputRoundListener());
-                        synchronized(synchReadLock){
                             try{
                                 if(inputTimeout == Integer.MAX_VALUE){
                                     //timeout disabled, wait forever or till the buffer is fullfilled
@@ -654,14 +676,15 @@
                                 }
                             }catch(InterruptedException iE){
                                 throw new IOException();
+                            } finally {
+                                stopReading();
                             }
-                        } // synch
-                        stopReading();
                     }
                 } // if !event thread
-                ret = dst.position() - ret;
-            } // if has Remaining
-        } // synch handle
+            } // synch
+            ret = dst.position() - ret;
+        } // if has Remaining
+
         return ret;
     }
 
@@ -677,41 +700,39 @@
             );
         }
 
-        synchronized (this) {
-            checkPowerState();
+        checkPowerState();
+
+        int ret = 0;
+
+        synchronized(synchWriteLock) {
             checkWrite();
+            isWriting = true;
         }
 
-        int ret = 0;
         try {
-            isWriting = true;
             /*
              * synchronous write0 returns number of written bytes
              * slice is needed to avoid memory corruption because src buffer modification
              * might happen during write0
              */
             ret = write0(src.slice());
-            try{
-                src.position(src.position() + ret);
-            }catch(IllegalArgumentException e){
-                //IAE happens if src.position() + ret < 0 (not expected) or src.position() + ret > limit
-                src.position(src.limit());
-            }
+            shiftBufferPosition(src, src.position() + ret);
         } finally {
             isWriting = false;
-            return ret;
         }
+        return ret;
     }
 
     @Override
     public synchronized void close() throws IOException{
         if (isOpen()) {
-            synchronized(synchReadLock){
-                synchReadLock.notifyAll();
-            }
             stopWriting();
             stopReading();
             super.close();
+            // unblock read()
+            synchronized(synchReadLock) {
+                synchReadLock.notifyAll();
+            }
         }
     }