A few synchronization issues identified and fixed

- SimpleDateFormat and DecimalFormat are not thread-safe, they need to be synchronized externally (cloning in this case) - StringBuffer is not needed in local variables, replaced with StringBuilder - CopyOnWriteArraySet is thread-safe, no need to synchronize method which is touching it
exxeleron · Mar 5, 2015 · 3cb4508 · 3cb4508
1 parent 6be0246
commit 3cb4508
Show file tree

Hide file tree

Showing 9 changed files with 23 additions and 27 deletions.
diff --git a/src/main/java/com/exxeleron/qjava/QCallbackConnection.java b/src/main/java/com/exxeleron/qjava/QCallbackConnection.java
@@ -83,11 +83,7 @@ public void close() throws IOException {
      * already exists, nothing happens.
      */
     public synchronized void startListener() {
-        if ( messageListener == null ) {
-            messageListener = new QListener();
-            listenerThread = new Thread(messageListener, "qJava-listener" + this.toString());
-            listenerThread.start();
-        }
+        startListener("qJava-listener" + this.toString());
     }
 
     /**
@@ -128,7 +124,7 @@ public synchronized void stopListener() {
      * @param listener
      *            a {@link QMessagesListener} to be registered
      */
-    public synchronized void addMessagesListener( final QMessagesListener listener ) {
+    public void addMessagesListener( final QMessagesListener listener ) {
         messagesListeners.add(listener);
     }
 
@@ -138,7 +134,7 @@ public synchronized void addMessagesListener( final QMessagesListener listener )
      * @param listener
      *            a {@link QMessagesListener} to be unregistered
      */
-    public synchronized void removeMessagesListener( final QMessagesListener listener ) {
+    public void removeMessagesListener( final QMessagesListener listener ) {
         messagesListeners.remove(listener);
     }
 

diff --git a/src/main/java/com/exxeleron/qjava/QDate.java b/src/main/java/com/exxeleron/qjava/QDate.java
@@ -135,7 +135,7 @@ public static QDate fromString( final String date ) {
         }
     }
 
-    private static synchronized DateFormat getDateformat() {
-        return dateFormat;
+    private static DateFormat getDateformat() {
+        return (DateFormat) dateFormat.clone();
     }
 }
diff --git a/src/main/java/com/exxeleron/qjava/QDateTime.java b/src/main/java/com/exxeleron/qjava/QDateTime.java
@@ -139,7 +139,7 @@ public static QDateTime fromString( final String date ) {
         }
     }
 
-    private static synchronized DateFormat getDateformat() {
-        return dateFormat;
+    private static DateFormat getDateformat() {
+        return (DateFormat) dateFormat.clone();
     }
 }
diff --git a/src/main/java/com/exxeleron/qjava/QDictionary.java b/src/main/java/com/exxeleron/qjava/QDictionary.java
@@ -175,7 +175,7 @@ public boolean equals( final Object obj ) {
         if ( areValuesArray ) {
             return Utils.deepArraysEquals(keys, d.keys) && Utils.deepArraysEquals(values, d.values);
         } else {
-            return Utils.deepArraysEquals(keys, d.keys) && ((QTable) values).equals(d.values);
+            return Utils.deepArraysEquals(keys, d.keys) && values.equals(d.values);
         }
     }
 

diff --git a/src/main/java/com/exxeleron/qjava/QMonth.java b/src/main/java/com/exxeleron/qjava/QMonth.java
@@ -144,7 +144,7 @@ public static QMonth fromString( final String date ) {
         }
     }
 
-    private static synchronized DateFormat getDateformat() {
-        return dateFormat;
+    private static DateFormat getDateformat() {
+        return (DateFormat) dateFormat.clone();
     }
 }
diff --git a/src/main/java/com/exxeleron/qjava/QTimespan.java b/src/main/java/com/exxeleron/qjava/QTimespan.java
@@ -102,7 +102,7 @@ public Date toDateTime() {
     public String toString() {
         final Date dt = toDateTime();
         return dt == null ? NULL_STR : (value < 0 ? "-" : "") + (Math.abs(value) / NANOS_PER_DAY) + getDateformat().format(dt)
-                + nanosFormatter.format(Math.abs(value) % NANOS_PER_SECOND);
+                + getNanosformat().format(Math.abs(value) % NANOS_PER_SECOND);
     }
 
     /**
@@ -159,11 +159,11 @@ public static QTimespan fromString( final String date ) {
         }
     }
 
-    private static synchronized DateFormat getDateformat() {
-        return dateFormat;
+    private static DateFormat getDateformat() {
+        return (DateFormat) dateFormat.clone();
     }
 
-    private static synchronized NumberFormat getNanosformat() {
-        return nanosFormatter;
+    private static NumberFormat getNanosformat() {
+        return (NumberFormat) nanosFormatter.clone();
     }
 }
diff --git a/src/main/java/com/exxeleron/qjava/QTimestamp.java b/src/main/java/com/exxeleron/qjava/QTimestamp.java
@@ -96,7 +96,7 @@ public Date toDateTime() {
     @Override
     public String toString() {
         final Date dt = toDateTime();
-        return dt == null ? NULL_STR : getDateformat().format(dt) + nanosFormatter.format(value % NANOS_PER_SECOND);
+        return dt == null ? NULL_STR : getDateformat().format(dt) + getNanosformat().format(value % NANOS_PER_SECOND);
 
     }
 
@@ -154,11 +154,11 @@ public static QTimestamp fromString( final String date ) {
         }
     }
 
-    private static synchronized DateFormat getDateformat() {
-        return dateFormat;
+    private static DateFormat getDateformat() {
+        return (DateFormat) dateFormat.clone();
     }
 
-    private static synchronized NumberFormat getNanosformat() {
-        return nanosFormatter;
+    private static NumberFormat getNanosformat() {
+        return (NumberFormat) nanosFormatter.clone();
     }
 }
diff --git a/src/main/java/com/exxeleron/qjava/QWriter.java b/src/main/java/com/exxeleron/qjava/QWriter.java
@@ -544,8 +544,8 @@ public void writeLongBigEndian( final long value ) {
             final byte[] arr = new byte[] { (byte) ((value >> 56) & 0xff), (byte) ((value >> 48) & 0xff), (byte) ((value >> 40) & 0xff),
                                            (byte) ((value >> 32) & 0xff), (byte) ((value >> 24) & 0xff), (byte) ((value >> 16) & 0xff),
                                            (byte) ((value >> 8) & 0xff), (byte) ((value >> 0) & 0xff) };
-            for ( int i = 0; i < arr.length; i++ ) {
-                writeByte(arr[i]);
+            for (byte anArr : arr) {
+                writeByte(anArr);
             }
         }
 

diff --git a/src/main/java/com/exxeleron/qjava/Utils.java b/src/main/java/com/exxeleron/qjava/Utils.java
@@ -54,7 +54,7 @@ static String arrayToString( final Object list ) {
             return "[]";
         } else {
             final int length = Array.getLength(list);
-            final StringBuffer buffer = new StringBuffer("[");
+            final StringBuilder buffer = new StringBuilder("[");
 
             Object obj = Array.get(list, 0);
             buffer.append(obj == null ? null : obj.getClass().isArray() ? arrayToString(obj) : obj);