changeset 266:9e33128d2ca4

8145419: I2CCombinedMessage.transfer() would not trigger a stop condition if last recently added buffer's remaining() == 0 Summary: last buffer such as buffer.remaining() != 0 is marked with respective attribute to trigger a stop condition. Besides, an unexpected IAE doesn't occur (JDK-8144994?) anymore. Reviewed-by: snazarki
author onazarkina
date Mon, 28 Dec 2015 15:07:18 +0300
parents 958f53876e01
children 2ebc0471aa02
files src/share/classes/com/oracle/dio/i2cbus/impl/I2CCombinedMessage.java src/share/linux/native/com/oracle/dio/i2c/i2c.c src/share/linux/native/com/oracle/dio/spibus/spi.c
diffstat 3 files changed, 109 insertions(+), 138 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/classes/com/oracle/dio/i2cbus/impl/I2CCombinedMessage.java	Sun Dec 13 18:44:33 2015 +0300
+++ b/src/share/classes/com/oracle/dio/i2cbus/impl/I2CCombinedMessage.java	Mon Dec 28 15:07:18 2015 +0300
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013, 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 2015, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -28,7 +28,6 @@
 import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
-import java.util.Objects;
 
 import com.oracle.dio.utils.ExceptionMessage;
 import com.oracle.dio.utils.Logging;
@@ -44,12 +43,14 @@
     int messageBus = DeviceConfig.DEFAULT;
     boolean isAlreadyTransferedOnce;
     int rxMessageCount;
+    int stopConditionIndex = -1;
 
     private class Message<P extends I2CDevice> {
         public P device;
         public ByteBuffer buf;
         public int skip;
         public boolean isRx;
+
         public Message(P device, ByteBuffer buf, int skip, boolean isRx) {
             this.device = device;
             this.buf = buf;
@@ -58,45 +59,58 @@
         }
     }
 
-    void check(Message message) throws ClosedDeviceException {
+    void validateAndAppend(Message message) throws ClosedDeviceException {
+        synchronized (messageList) {
+            if (isAlreadyTransferedOnce) {
+                throw new IllegalStateException(
+                        ExceptionMessage.format(ExceptionMessage.I2CBUS_ALREADY_TRANSFERRED_MESSAGE)
+                );
+            }
 
-        if (isAlreadyTransferedOnce) {
-            throw new IllegalStateException(
-                ExceptionMessage.format(ExceptionMessage.I2CBUS_ALREADY_TRANSFERRED_MESSAGE)
-            );
-        }
+            if (0 > message.skip) {
+                throw new IllegalArgumentException(
+                        ExceptionMessage.format(ExceptionMessage.I2CBUS_NEGATIVE_SKIP_ARG)
+                );
+            }
 
-        // null check
-        message.buf.capacity();
-
-        if (0 > message.skip) {
-            throw new IllegalArgumentException(
-                ExceptionMessage.format(ExceptionMessage.I2CBUS_NEGATIVE_SKIP_ARG)
-            );
-        }
-
-        if (!message.device.isOpen()) {
-            throw new ClosedDeviceException();
-        }
+            if (!message.device.isOpen()) {
+                throw new ClosedDeviceException();
+            }
 
         /*  check that can get '-1' here */
-        int busNumber = ((I2CDeviceConfig) message.device.getDescriptor().getConfiguration()).getControllerNumber();
+            int busNumber = ((I2CDeviceConfig) message.device.getDescriptor().getConfiguration()).getControllerNumber();
 
-        if (DeviceConfig.DEFAULT == messageBus) {
-            messageBus = busNumber;
-        } else {
-            if (messageBus != busNumber) {
-                throw new IllegalArgumentException(
-                    ExceptionMessage.format(ExceptionMessage.I2CBUS_DIFFERENT_BUS_SLAVE_OPERATION)
-                );
+            if (DeviceConfig.DEFAULT == messageBus) {
+                messageBus = busNumber;
+            } else {
+                if (messageBus != busNumber) {
+                    throw new IllegalArgumentException(
+                            ExceptionMessage.format(ExceptionMessage.I2CBUS_DIFFERENT_BUS_SLAVE_OPERATION)
+                    );
+                }
             }
-        }
 
-        for (int i = 0; i < messageList.size(); i++) {
-            if (messageList.get(i).buf == message.buf) {
-                throw new IllegalArgumentException(
-                    ExceptionMessage.format(ExceptionMessage.I2CBUS_BUFFER_GIVEN_TWICE)
-                );
+            // null check and mark for stopCondition.
+            if (message.buf.remaining() != 0) {
+                boolean alreadyAppended = false;
+                for (Message<I2CSlaveImpl> already : messageList) {
+                    if (already.buf == message.buf) {
+                        alreadyAppended = true;
+                        break;
+                    }
+                }
+
+                // if the buffer is used several times, stop condition must occur only once,
+                // on first use,
+                if (!alreadyAppended)
+                    stopConditionIndex = messageList.size();
+            }
+
+
+            messageList.add(message);
+
+            if (message.isRx) {
+                ++rxMessageCount;
             }
         }
     }
@@ -111,28 +125,17 @@
      * Appends a read message/operation from the provided I2C slave device. Reads up to
      * {@code rwBuf.remaining()} bytes of data from this slave device into the buffer {@code rxBuf}.
      *
-     * @param slave
-     *            the I2C slave device to read from.
-     * @param rxBuf
-     *            the buffer into which the data is read.
+     * @param slave the I2C slave device to read from.
+     * @param rxBuf the buffer into which the data is read.
      * @return a reference to this {@code I2CCombinedMessage} object.
-     * @throws NullPointerException
-     *             If {@code rxBuf} is {@code null}.
-     * @throws IllegalStateException
-     *             if this message has already been transferred once.
-     * @throws ClosedDeviceException
-     *             if the device has been closed.
-     * @throws IllegalArgumentException
-     *             if appending the read operation to a slave on a different bus.
+     * @throws NullPointerException     If {@code rxBuf} is {@code null}.
+     * @throws IllegalStateException    if this message has already been transferred once.
+     * @throws ClosedDeviceException    if the device has been closed.
+     * @throws IllegalArgumentException if appending the read operation to a slave on a different bus.
      */
     public I2CCombinedMessage appendRead(I2CDevice slave, ByteBuffer rxBuf)
             throws ClosedDeviceException {
-        Message message = new Message(slave, rxBuf, 0, true);
-        synchronized (this) {
-            check(message);
-            messageList.add(message);
-            ++rxMessageCount;
-        }
+        validateAndAppend(new Message(slave, rxBuf, 0, true));
         return this;
     }
 
@@ -141,34 +144,21 @@
      * {@code rwBuf.remaining()} bytes of data from this slave device into the buffer skipping
      * {@code rxBuf} the first {@code rxSkip} bytes read.
      *
-     * @param slave
-     *            the I2C slave device to read from.
-     * @param rxSkip
-     *            the number of read bytes that must be ignored/skipped before filling in the
-     *            {@code rxBuf} buffer.
-     * @param rxBuf
-     *            the buffer into which the data is read.
+     * @param slave  the I2C slave device to read from.
+     * @param rxSkip the number of read bytes that must be ignored/skipped before filling in the
+     *               {@code rxBuf} buffer.
+     * @param rxBuf  the buffer into which the data is read.
      * @return a reference to this {@code I2CCombinedMessage} object.
-     * @throws IOException
-     *             if some other I/O error occurs.
-     * @throws NullPointerException
-     *             If {@code rxBuf} is {@code null}.
-     * @throws IllegalStateException
-     *             if this message has already been transferred once.
-     * @throws ClosedDeviceException
-     *             if the device has been closed.
-     * @throws IllegalArgumentException
-     *             if {@code rxSkip} is negative or if appending the read operation to a slave on a
-     *             different bus.
+     * @throws IOException              if some other I/O error occurs.
+     * @throws NullPointerException     If {@code rxBuf} is {@code null}.
+     * @throws IllegalStateException    if this message has already been transferred once.
+     * @throws ClosedDeviceException    if the device has been closed.
+     * @throws IllegalArgumentException if {@code rxSkip} is negative or if appending the read operation to a slave on a
+     *                                  different bus.
      */
     public I2CCombinedMessage appendRead(I2CDevice slave, int rxSkip,
-            ByteBuffer rxBuf) throws IOException, ClosedDeviceException {
-        Message message = new Message(slave, rxBuf, rxSkip, true);
-        synchronized (this) {
-            check(message);
-            messageList.add(message);
-            ++rxMessageCount;
-        }
+                                         ByteBuffer rxBuf) throws IOException, ClosedDeviceException {
+        validateAndAppend(new Message(slave, rxBuf, rxSkip, true));
         return this;
     }
 
@@ -176,32 +166,20 @@
      * Appends a write message/operation from the provided I2C slave device. Writes to this slave
      * device {@code txBuff.remaining()} bytes from the buffer {@code txBuf}.
      *
-     * @param slave
-     *            the I2C slave device to write to.
-     * @param txBuf
-     *            the buffer containing the bytes to write.
+     * @param slave the I2C slave device to write to.
+     * @param txBuf the buffer containing the bytes to write.
      * @return a reference to this {@code I2CCombinedMessage} object.
-     * @throws IOException
-     *             if some other I/O error occurs.
-     * @throws NullPointerException
-     *             If {@code txBuf} is {@code null}.
-     * @throws IndexOutOfBoundsException
-     *             {@code txOff} or {@code txLen} points or results in pointing outside
-     *             {@code txBuf}.
-     * @throws IllegalStateException
-     *             if this message has already been transferred once.
-     * @throws ClosedDeviceException
-     *             if the device has been closed.
-     * @throws IllegalArgumentException
-     *             if appending the write operation to a slave on a different bus.
+     * @throws IOException               if some other I/O error occurs.
+     * @throws NullPointerException      If {@code txBuf} is {@code null}.
+     * @throws IndexOutOfBoundsException {@code txOff} or {@code txLen} points or results in pointing outside
+     *                                   {@code txBuf}.
+     * @throws IllegalStateException     if this message has already been transferred once.
+     * @throws ClosedDeviceException     if the device has been closed.
+     * @throws IllegalArgumentException  if appending the write operation to a slave on a different bus.
      */
     public I2CCombinedMessage appendWrite(I2CDevice slave, ByteBuffer txBuf) throws IOException,
             ClosedDeviceException {
-        Message message = new Message(slave, txBuf, 0, false);
-        synchronized (this) {
-            check(message);
-            messageList.add(message);
-        }
+        validateAndAppend(new Message(slave, txBuf, 0, false));
         return this;
     }
 
@@ -209,64 +187,57 @@
      * Transfers this combined message. This will result in each of the contained
      * messages/operations to be sent/executed in the same order they have been appended to this
      * combined message.
-     * <p />
+     * <p>
      * Once transfer no additional operation can be appended anymore to this combined message. Any
      * such attempt will result in a {@link IllegalStateException} to be thrown. <br />
      * This combined message can be transferred several times.
      *
      * @return an array containing the number of bytes read for each of the read operations of this
-     *         combined message; the results of each read operations appear in the very same order
-     *         the read operations have been appended to this combined message.
-     * @throws IOException
-     *             if an I/O error occurred
-     * @throws UnavailableDeviceException
-     *             if this device is not currently available - such as it is locked by another
-     *             application.
-     * @throws ClosedDeviceException
-     *             if any of the targeted devices is not currently available (has been closed).
+     * combined message; the results of each read operations appear in the very same order
+     * the read operations have been appended to this combined message.
+     * @throws IOException                if an I/O error occurred
+     * @throws UnavailableDeviceException if this device is not currently available - such as it is locked by another
+     *                                    application.
+     * @throws ClosedDeviceException      if any of the targeted devices is not currently available (has been closed).
      */
     public int[] transfer() throws IOException, UnavailableDeviceException, ClosedDeviceException {
         int bytesRead[];
-        synchronized (this) {
+        synchronized (messageList) {
             /* Forbid adding more messages to this combined message */
             isAlreadyTransferedOnce = true;
             if (0 == messageList.size()) {
-                notifyAll();
                 return null;
             }
 
             bytesRead = new int[rxMessageCount];
             int bytesReadIdx = 0;
 
-            try {
-                final int size = messageList.size();
-                if (1 == size) {
-                    Message message = messageList.get(0);
+            final int size = messageList.size();
+
+            if (1 == size) {
+                Message message = messageList.get(0);
+                int skip = (message.isRx) ? message.skip : -1;
+                Logging.reportInformation(ExceptionMessage.format(ExceptionMessage.I2CBUS_LAST_MESSAGE));
+                int res = ((I2CSlaveImpl) message.device).transfer(I2CSlaveImpl.I2C_REGULAR, skip, message.buf);
+                if (rxMessageCount > 0) {
+                    bytesRead[0] = res;
+                }
+            } else {
+                int flag = I2CSlaveImpl.I2C_COMBINED_START;
+                Logging.reportInformation(ExceptionMessage.format(ExceptionMessage.I2CBUS_FIRST_MESSAGE));
+                for (int i = 0; i < messageList.size(); i++) {
+                    Message message = messageList.get(i);
                     int skip = (message.isRx) ? message.skip : -1;
-                    Logging.reportInformation(ExceptionMessage.format(ExceptionMessage.I2CBUS_LAST_MESSAGE));
-                    int res = ((I2CSlaveImpl)message.device).transfer(I2CSlaveImpl.I2C_REGULAR, skip, message.buf);
-                    if (rxMessageCount > 0) {
-                        bytesRead[0] = res;
+                    if (i == stopConditionIndex) {
+                        Logging.reportInformation(ExceptionMessage.format(ExceptionMessage.I2CBUS_LAST_MESSAGE));
+                        flag = I2CSlaveImpl.I2C_COMBINED_END;
                     }
-                } else {
-                    int flag = I2CSlaveImpl.I2C_COMBINED_START;
-                    Logging.reportInformation(ExceptionMessage.format(ExceptionMessage.I2CBUS_FIRST_MESSAGE));
-                    for (int i = 0; i < messageList.size(); i++) {
-                        Message message = messageList.get(i);
-                        int skip = (message.isRx) ? message.skip : -1;
-                        if(i == messageList.size() - 1) {
-                            Logging.reportInformation(ExceptionMessage.format(ExceptionMessage.I2CBUS_LAST_MESSAGE));
-                            flag = I2CSlaveImpl.I2C_COMBINED_END;
-                        }
-                        int res = ((I2CSlaveImpl)message.device).transfer(flag, skip, message.buf);
-                        if (message.isRx) {
-                            bytesRead[bytesReadIdx++] = res;
-                        }
-                        flag = I2CSlaveImpl.I2C_COMBINED_BODY;
+                    int res = ((I2CSlaveImpl) message.device).transfer(flag, skip, message.buf);
+                    if (message.isRx) {
+                        bytesRead[bytesReadIdx++] = res;
                     }
+                    flag = I2CSlaveImpl.I2C_COMBINED_BODY;
                 }
-            } finally {
-                notifyAll();
             }
 
         }
--- a/src/share/linux/native/com/oracle/dio/i2c/i2c.c	Sun Dec 13 18:44:33 2015 +0300
+++ b/src/share/linux/native/com/oracle/dio/i2c/i2c.c	Mon Dec 28 15:07:18 2015 +0300
@@ -406,7 +406,7 @@
         pthread_detach(pDev->context);
     } else {
         if (0 != pthread_join(pDev->context, (void**)pBytes)) {
-            JAVACALL_REPORT_ERROR1(JC_DIO, "[I2C] Can't joint thread: %d", errno);
+            JAVACALL_REPORT_ERROR1(JC_DIO, "[I2C] Can't join thread: %d", errno);
         }
     }
 
--- a/src/share/linux/native/com/oracle/dio/spibus/spi.c	Sun Dec 13 18:44:33 2015 +0300
+++ b/src/share/linux/native/com/oracle/dio/spibus/spi.c	Mon Dec 28 15:07:18 2015 +0300
@@ -451,7 +451,7 @@
     lock.l_pid    = getpid();
 
     if(-1 == fcntl(cfg->devFd , F_SETLK, &lock)){
-        JAVACALL_REPORT_WARN1(JC_DIO, "[SPI] Can't unclock device. errno %d", errno);
+        JAVACALL_REPORT_WARN1(JC_DIO, "[SPI] Can't unlock device. errno %d", errno);
     }
 
     close( cfg->devFd );