From 00d90317d8e7e1586eb1e30ff9643ccf0b20b469 Mon Sep 17 00:00:00 2001 From: Wei-Chiu Chuang Date: Sat, 3 Jul 2021 00:45:21 +0800 Subject: [PATCH] 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. --- .../phoenix/jdbc/PhoenixConnection.java | 29 ++++--------------- .../apache/phoenix/log/TableLogWriter.java | 2 +- .../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 da704420d37..105f45b6999 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 @@ -177,8 +177,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 childConnections = null; + private final ConcurrentLinkedQueue 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. @@ -467,18 +467,10 @@ public boolean isInternalConnection() { } /** - * 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); } @@ -488,9 +480,7 @@ public void addChildConnection(PhoenixConnection connection) { * @param connection */ public void removeChildConnection(PhoenixConnection connection) { - if (childConnections != null) { - childConnections.remove(connection); - } + childConnections.remove(connection); } /** @@ -500,10 +490,7 @@ public void removeChildConnection(PhoenixConnection connection) { */ @VisibleForTesting public int getChildConnectionsCount() { - if (childConnections != null) { - return childConnections.size(); - } - return 0; + return childConnections.size(); } public Sampler getSampler() { @@ -721,11 +708,7 @@ public void close() throws SQLException { 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 966bed42a89..dc610b556bc 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 @@ */ 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 dd1f6eca986..bccf7bf5422 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 @@ private static class PNameImplData { /** */ public byte[] bytesName; /** */ - public ImmutableBytesPtr ptr; + public volatile ImmutableBytesPtr ptr; /** *