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; /** *