Skip to content
Permalink
Browse files

0003959: Possible race conditions in parameters, MongoDB, and initial…

…izing nodes -- Fix double check and synchronization (#96)

* fix double check and synchronization

* more synchronization issues

* more doublecheck bugs
  • Loading branch information...
davecramer authored and jaredfrees committed May 15, 2019
1 parent a0d6ad1 commit 53a7207671e5ac64adaec310fa8728ca7db5afaf
@@ -41,7 +41,7 @@
* This is static because the MongoClient is thread safe and wraps a pool of
* connections
*/
protected static Map<String, MongoClient> clients = new HashMap<String, MongoClient>();
protected final static Map<String, MongoClient> clients = new HashMap<String, MongoClient>();

protected DB currentDB;

@@ -51,32 +51,28 @@ public SimpleMongoClientManager(IParameterService parameterService, String name)
}

@Override
public MongoClient get() {
public synchronized MongoClient get() {
MongoClient client = clients.get(name);
if (client == null) {
synchronized (clients) {
if (client == null) {
int port = 27017;
String host = "localhost";
if (parameterService != null) {
port = parameterService.getInt(name + MongoConstants.PORT, port);
host = parameterService.getString(name + MongoConstants.HOST, host);
}
int port = 27017;
String host = "localhost";
if (parameterService != null) {
port = parameterService.getInt(name + MongoConstants.PORT, port);
host = parameterService.getString(name + MongoConstants.HOST, host);
}

try {
client = new MongoClient(host, port);
clients.put(name, client);
} catch (UnknownHostException e) {
throw new SymmetricException(e);
}
}
try {
client = new MongoClient(host, port);
clients.put(name, client);
} catch (UnknownHostException e) {
throw new SymmetricException(e);
}
}
return client;
}

@Override
public DB getDB(String name) {
public synchronized DB getDB(String name) {
if (currentDB == null || !currentDB.getName().equals(name)) {
currentDB = get().getDB(name);
/**
@@ -156,28 +156,23 @@ public synchronized void rereadParameters() {
getParameters();
}

protected TypedProperties getParameters() {
protected synchronized TypedProperties getParameters() {
long timeoutTime = System.currentTimeMillis() - cacheTimeoutInMs;
// Quick check if cache is timed out
// see if the parameters have timed out
if (parameters == null || (cacheTimeoutInMs > 0 && lastTimeParameterWereCached < timeoutTime)) {
synchronized (this) {
// Synchronized check to prevent calling reread parameters multiple times
if (parameters == null || (cacheTimeoutInMs > 0 && lastTimeParameterWereCached < timeoutTime)) {
try {
parameters = rereadApplicationParameters();
SymmetricUtils.replaceSystemAndEnvironmentVariables(parameters);
lastTimeParameterWereCached = System.currentTimeMillis();
cacheTimeoutInMs = getInt(ParameterConstants.PARAMETER_REFRESH_PERIOD_IN_MS);
} catch (SqlException ex) {
if (parameters != null) {
log.warn("Could not read database parameters. We will try again later", ex);
} else {
log.error("Could not read database parameters and they have not yet been initialized");
throw ex;
}
throw ex;
}
try {
parameters = rereadApplicationParameters();
SymmetricUtils.replaceSystemAndEnvironmentVariables(parameters);
lastTimeParameterWereCached = System.currentTimeMillis();
cacheTimeoutInMs = getInt(ParameterConstants.PARAMETER_REFRESH_PERIOD_IN_MS);
} catch (SqlException ex) {
if (parameters != null) {
log.warn("Could not read database parameters. We will try again later", ex);
} else {
log.error("Could not read database parameters and they have not yet been initialized");
throw ex;
}
throw ex;
}
}
return parameters;
@@ -192,7 +187,7 @@ public Date getLastTimeParameterWereCached() {
}


public String getExternalId() {
public synchronized String getExternalId() {
if (externalId==null) {
String value = getString(ParameterConstants.EXTERNAL_ID);
value = substituteScripts(value);
@@ -201,10 +196,10 @@ public String getExternalId() {
log.debug("External Id eval results in: {}",externalId);
}
}
return externalId;
return externalId;
}

public String getSyncUrl() {
public synchronized String getSyncUrl() {
if (syncUrl==null) {
String value = getString(ParameterConstants.SYNC_URL);
value = substituteScripts(value);
@@ -216,10 +211,10 @@ public String getSyncUrl() {
log.debug("Sync URL eval results in: {}",syncUrl);
}
}
return syncUrl;
return syncUrl;
}

public String getNodeGroupId() {
public synchronized String getNodeGroupId() {
if (nodeGroupId==null) {
String value = getString(ParameterConstants.NODE_GROUP_ID);
value = substituteScripts(value);
@@ -228,10 +223,10 @@ public String getNodeGroupId() {
log.debug("Node Group Id eval results in: {}",nodeGroupId);
}
}
return nodeGroupId;
return nodeGroupId;
}

public String getRegistrationUrl() {
public synchronized String getRegistrationUrl() {
if (registrationUrl==null) {
String value = getString(ParameterConstants.REGISTRATION_URL);
value = substituteScripts(value);
@@ -243,10 +238,10 @@ public String getRegistrationUrl() {
log.debug("Registration URL eval results in: {}",registrationUrl);
}
}
return registrationUrl;
return registrationUrl;
}

public String getEngineName() {
public synchronized String getEngineName() {
if (engineName==null) {
String value = getString(ParameterConstants.ENGINE_NAME,"SymmetricDS");
value = substituteScripts(value);
@@ -268,7 +263,7 @@ public String getEngineName() {
return replacementValues;
}

public void setDatabaseHasBeenInitialized(boolean databaseHasBeenInitialized) {
public synchronized void setDatabaseHasBeenInitialized(boolean databaseHasBeenInitialized) {
if (this.databaseHasBeenInitialized != databaseHasBeenInitialized) {
this.databaseHasBeenInitialized = databaseHasBeenInitialized;
this.parameters = null;
@@ -277,7 +272,7 @@ public void setDatabaseHasBeenInitialized(boolean databaseHasBeenInitialized) {

abstract public TypedProperties getDatabaseParameters(String externalId, String nodeGroupId);

protected TypedProperties rereadDatabaseParameters(Properties p) {
protected synchronized TypedProperties rereadDatabaseParameters(Properties p) {
if (databaseHasBeenInitialized) {
TypedProperties properties = getDatabaseParameters(ParameterConstants.ALL,
ParameterConstants.ALL);
@@ -286,7 +281,6 @@ protected TypedProperties rereadDatabaseParameters(Properties p) {
properties.putAll(getDatabaseParameters(
p.getProperty(ParameterConstants.EXTERNAL_ID),
p.getProperty(ParameterConstants.NODE_GROUP_ID)));
databaseHasBeenInitialized = true;
return properties;
} else {
return new TypedProperties();
@@ -96,25 +96,21 @@ public NodeCommunicationService(IClusterService clusterService, INodeService nod
}
}

private final void initialize() {
private synchronized final void initialize() {
if (!initialized) {
synchronized (this) {
if (!initialized) {
if (clusterService.isClusteringEnabled()) {
try {
int locksCleared = sqlTemplate.update(getSql("clearLocksOnRestartSql"),
clusterService.getServerId());
if (locksCleared > 0) {
log.info("Cleared {} node communication locks for {}", locksCleared,
clusterService.getServerId());
}
} finally {
initialized = true;
}
} else {
initialized = true;
if (clusterService.isClusteringEnabled()) {
try {
int locksCleared = sqlTemplate.update(getSql("clearLocksOnRestartSql"),
clusterService.getServerId());
if (locksCleared > 0) {
log.info("Cleared {} node communication locks for {}", locksCleared,
clusterService.getServerId());
}
} finally {
initialized = true;
}
} else {
initialized = true;
}
}
}

0 comments on commit 53a7207

Please sign in to comment.
You can’t perform that action at this time.