This is an automated email from the ASF dual-hosted git repository.

stoty pushed a commit to branch 5.1
in repository https://gitbox.apache.org/repos/asf/phoenix.git


The following commit(s) were added to refs/heads/5.1 by this push:
     new c5dfb0f  PHOENIX-6510 Double-Checked Locking field must be volatile
c5dfb0f is described below

commit c5dfb0f6a26b02459d0e6201597b354a380ed668
Author: Wei-Chiu Chuang <weic...@apache.org>
AuthorDate: Sat Jul 3 00:45:21 2021 +0800

    PHOENIX-6510 Double-Checked Locking field must be volatile
    
    added volatile keyword to the variable to be doubly checked.
    Do not use double check pattern in PhoenixConnection.
---
 .../org/apache/phoenix/jdbc/PhoenixConnection.java | 29 +++++-----------------
 .../org/apache/phoenix/log/TableLogWriter.java     |  2 +-
 .../java/org/apache/phoenix/schema/PNameImpl.java  |  2 +-
 3 files changed, 8 insertions(+), 25 deletions(-)

diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java 
b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java
index 74aefbe..b4b3d7a 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java
@@ -176,8 +176,8 @@ public class PhoenixConnection implements Connection, 
MetaDataMutated, SQLClosea
     private Double logSamplingRate;
     private String sourceOfOperation;
 
-    private Object queueCreationLock = new Object(); // lock for the lazy init 
path of childConnections structure
-    private ConcurrentLinkedQueue<PhoenixConnection> childConnections = null;
+    private final ConcurrentLinkedQueue<PhoenixConnection> childConnections =
+        new ConcurrentLinkedQueue<>();
 
     //For now just the copy constructor paths will have this as true as I 
don't want to change the
     //public interfaces.
@@ -466,18 +466,10 @@ public class PhoenixConnection implements Connection, 
MetaDataMutated, SQLClosea
     }
 
     /**
-     * This method, and *only* this method is thread safe
+     * Add connection to the internal childConnections queue
      * @param connection
      */
     public void addChildConnection(PhoenixConnection connection) {
-        //double check for performance
-        if(childConnections == null) {
-            synchronized (queueCreationLock) {
-                if (childConnections == null) {
-                    childConnections = new ConcurrentLinkedQueue<>();
-                }
-            }
-        }
         childConnections.add(connection);
     }
 
@@ -487,9 +479,7 @@ public class PhoenixConnection implements Connection, 
MetaDataMutated, SQLClosea
      * @param connection
      */
     public void removeChildConnection(PhoenixConnection connection) {
-        if (childConnections != null) {
-            childConnections.remove(connection);
-        }
+        childConnections.remove(connection);
     }
 
     /**
@@ -499,10 +489,7 @@ public class PhoenixConnection implements Connection, 
MetaDataMutated, SQLClosea
      */
     @VisibleForTesting
     public int getChildConnectionsCount() {
-        if (childConnections != null) {
-            return childConnections.size();
-        }
-        return 0;
+        return childConnections.size();
     }
 
     public Sampler<?> getSampler() {
@@ -719,11 +706,7 @@ public class PhoenixConnection implements Connection, 
MetaDataMutated, SQLClosea
                     traceScope.close();
                 }
                 closeStatements();
-                synchronized (queueCreationLock) {
-                    if (childConnections != null) {
-                        SQLCloseables.closeAllQuietly(childConnections);
-                    }
-                }
+                SQLCloseables.closeAllQuietly(childConnections);
             } finally {
                 services.removeConnection(this);
             }
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/log/TableLogWriter.java 
b/phoenix-core/src/main/java/org/apache/phoenix/log/TableLogWriter.java
index 966bed4..dc610b5 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/log/TableLogWriter.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/log/TableLogWriter.java
@@ -41,7 +41,7 @@ import 
org.apache.phoenix.thirdparty.com.google.common.collect.ImmutableMap;
  */
 public class TableLogWriter implements LogWriter {
     private static final Logger LOGGER = 
LoggerFactory.getLogger(LogWriter.class);
-    private Connection connection;
+    private volatile Connection connection;
     private boolean isClosed;
     private PreparedStatement upsertStatement;
     private Configuration config;
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/schema/PNameImpl.java 
b/phoenix-core/src/main/java/org/apache/phoenix/schema/PNameImpl.java
index dd1f6ec..bccf7bf 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/schema/PNameImpl.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/PNameImpl.java
@@ -33,7 +33,7 @@ public class PNameImpl implements PName {
         /**  */
         public byte[] bytesName;
         /**  */
-        public ImmutableBytesPtr ptr;
+        public volatile ImmutableBytesPtr ptr;
 
         /**
          *

Reply via email to