This is an automated email from the ASF dual-hosted git repository.
nnag pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 97f5f91 GEODE-6470: Making double-checked locking thread-safe (#3252)
97f5f91 is described below
commit 97f5f9157843e2832ce52cae131d642c923c0e9a
Author: Nabarun Nag <[email protected]>
AuthorDate: Fri Mar 1 09:32:18 2019 -0800
GEODE-6470: Making double-checked locking thread-safe (#3252)
* A repeated check on a non-volatile field is not thread-safe, and
could result in unexpected behavior.
* Fixed using volatile and local variables
---
.../internal/cache/wan/GatewaySenderEventImpl.java | 2 +-
.../datasource/GemFireBasicDataSource.java | 28 ++++++++++++----------
.../geode/management/internal/cli/shell/Gfsh.java | 13 ++++++----
3 files changed, 25 insertions(+), 18 deletions(-)
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/wan/GatewaySenderEventImpl.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/wan/GatewaySenderEventImpl.java
index 4a1e802..8db48b4 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/wan/GatewaySenderEventImpl.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/wan/GatewaySenderEventImpl.java
@@ -117,7 +117,7 @@ public class GatewaySenderEventImpl
/**
* The serialized new value for this event's key. May not be computed at
construction time.
*/
- protected byte[] value;
+ protected volatile byte[] value;
/**
* The "object" form of the value. Will be null after this object is
deserialized.
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/datasource/GemFireBasicDataSource.java
b/geode-core/src/main/java/org/apache/geode/internal/datasource/GemFireBasicDataSource.java
index 34404d7..163c847 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/datasource/GemFireBasicDataSource.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/datasource/GemFireBasicDataSource.java
@@ -36,7 +36,7 @@ public class GemFireBasicDataSource extends
AbstractDataSource {
private static final long serialVersionUID = -4010116024816908360L;
/** Creates a new instance of BaseDataSource */
- protected transient Driver driverObject = null;
+ protected transient volatile Driver driverObject = null;
/**
* Place holder for abstract method isWrapperFor(java.lang.Class) in
java.sql.Wrapper required by
@@ -68,7 +68,7 @@ public class GemFireBasicDataSource extends
AbstractDataSource {
*/
public GemFireBasicDataSource(ConfiguredDataSourceProperties configs) throws
SQLException {
super(configs);
- loadDriver();
+ driverObject = loadDriver();
}
/**
@@ -83,10 +83,14 @@ public class GemFireBasicDataSource extends
AbstractDataSource {
// connection without username & password
// we should just return the desired connection
Connection connection = null;
- if (driverObject == null) {
+ Driver localDriverRef = driverObject;
+ if (localDriverRef == null) {
synchronized (this) {
- if (driverObject == null)
- loadDriver();
+ localDriverRef = driverObject;
+ if (localDriverRef == null) {
+ localDriverRef = loadDriver();
+ driverObject = localDriverRef;
+ }
}
}
@@ -128,12 +132,12 @@ public class GemFireBasicDataSource extends
AbstractDataSource {
return getConnection();
}
- private void loadDriver() throws SQLException {
+ private Driver loadDriver() throws SQLException {
try {
if (jdbcDriver != null && jdbcDriver.length() > 0) {
- loadDriverUsingClassName();
+ return loadDriverUsingClassName();
} else {
- loadDriverUsingURL();
+ return loadDriverUsingURL();
}
} catch (Exception ex) {
String msg =
@@ -144,13 +148,13 @@ public class GemFireBasicDataSource extends
AbstractDataSource {
}
}
- private void loadDriverUsingURL() throws SQLException {
- driverObject = DriverManager.getDriver(this.url);
+ private Driver loadDriverUsingURL() throws SQLException {
+ return DriverManager.getDriver(this.url);
}
- private void loadDriverUsingClassName()
+ private Driver loadDriverUsingClassName()
throws ClassNotFoundException, InstantiationException,
IllegalAccessException {
Class<?> driverClass = ClassPathLoader.getLatest().forName(jdbcDriver);
- driverObject = (Driver) driverClass.newInstance();
+ return (Driver) driverClass.newInstance();
}
}
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
index f75075a..ac38304 100755
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
@@ -137,7 +137,7 @@ public class Gfsh extends JLineShell {
protected static PrintStream gfsherr = System.err;
protected static final ThreadLocal<Gfsh> gfshThreadLocal = new
ThreadLocal<>();
@MakeNotStatic
- private static Gfsh instance;
+ private static volatile Gfsh instance;
// This flag is used to restrict column trimming to table only types
private static final ThreadLocal<Boolean> resultTypeTL = new ThreadLocal<>();
private static final String OS = System.getProperty("os.name").toLowerCase();
@@ -247,11 +247,14 @@ public class Gfsh extends JLineShell {
}
public static Gfsh getInstance(boolean launchShell, String[] args,
GfshConfig gfshConfig) {
- if (instance == null) {
+ Gfsh localGfshInstance = instance;
+ if (localGfshInstance == null) {
synchronized (INSTANCE_LOCK) {
- if (instance == null) {
- instance = new Gfsh(launchShell, args, gfshConfig);
- instance.executeInitFileIfPresent();
+ localGfshInstance = instance;
+ if (localGfshInstance == null) {
+ localGfshInstance = new Gfsh(launchShell, args, gfshConfig);
+ localGfshInstance.executeInitFileIfPresent();
+ instance = localGfshInstance;
}
}
}
