code.onehippo.org is currently readonly. We are migrating to code.bloomreach.com, please continue working there on Monday 14/12. See: https://docs.bloomreach.com/display/engineering/GitLab

Commit cc3e10d9 authored by Ard Schrijvers's avatar Ard Schrijvers

REPO-1811 Encapsulate the localLocks object

Since localLocks is a hashmap, all access to it should be synchronized.
This can be easier achieved by encapsulating it and whenever needed by
other classes, return a new object containing the same locks
parent dac8d9cc
......@@ -34,7 +34,7 @@ public abstract class AbstractLockManager implements InternalLockManager {
/**
* This locks object contains only the locks held by the *current* JVM
*/
protected final Map<String, MutableLock> localLocks = new HashMap<>();
private final Map<String, MutableLock> localLocks = new HashMap<>();
private final ScheduledExecutorService scheduledExecutorService;
......@@ -267,6 +267,13 @@ public abstract class AbstractLockManager implements InternalLockManager {
scheduledExecutorService.scheduleAtFixedRate(synchronizedRunnable, initialDelaySeconds, periodSeconds, SECONDS);
}
/**
* @return a copy of {@code localLocks}
*/
public synchronized Map<String, MutableLock> getLocalLocks() {
return new HashMap<>(localLocks);
}
public class UnlockStoppedThreadJanitor implements Runnable {
@Override
public void run() {
......
......@@ -97,7 +97,7 @@ public class DbLockManager extends AbstractLockManager {
addJob(new UnlockStoppedThreadJanitor());
addJob(new DbResetExpiredLocksJanitor(dataSource));
addJob(new DbLockRefresher(dataSource, clusterNodeId));
addJob(new LockThreadInterrupter(dataSource, clusterNodeId, localLocks));
addJob(new LockThreadInterrupter(dataSource, clusterNodeId, this));
}
@Override
......@@ -106,7 +106,7 @@ public class DbLockManager extends AbstractLockManager {
}
@Override
synchronized protected MutableLock createLock(final String key, final String threadName, final int refreshRateSeconds) throws LockException {
protected synchronized MutableLock createLock(final String key, final String threadName, final int refreshRateSeconds) throws LockException {
Connection connection = null;
boolean originalAutoCommit = false;
try {
......@@ -156,7 +156,7 @@ public class DbLockManager extends AbstractLockManager {
}
@Override
synchronized protected void releasePersistedLock(final String key, final String threadName) throws LockException {
protected synchronized void releasePersistedLock(final String key, final String threadName) throws LockException {
Connection connection = null;
boolean originalAutoCommit = false;
try {
......@@ -198,7 +198,7 @@ public class DbLockManager extends AbstractLockManager {
}
@Override
synchronized protected void abortPersistedLock(final String key) throws LockException {
protected synchronized void abortPersistedLock(final String key) throws LockException {
Connection connection = null;
boolean originalAutoCommit = false;
try {
......@@ -229,7 +229,7 @@ public class DbLockManager extends AbstractLockManager {
}
@Override
protected boolean containsLock(final String key) throws LockException {
protected synchronized boolean containsLock(final String key) throws LockException {
try (Connection connection = dataSource.getConnection()) {
final PreparedStatement selectStatement = connection.prepareStatement(SELECT_STATEMENT);
selectStatement.setString(1, key);
......@@ -255,7 +255,7 @@ public class DbLockManager extends AbstractLockManager {
}
@Override
protected List<Lock> retrieveLocks() throws LockException {
protected synchronized List<Lock> retrieveLocks() throws LockException {
try (Connection connection = dataSource.getConnection()) {
final PreparedStatement selectStatement = connection.prepareStatement(ALL_LOCKED_STATEMENT);
ResultSet resultSet = selectStatement.executeQuery();
......
......@@ -39,17 +39,18 @@ public class LockThreadInterrupter implements Runnable {
private final DataSource dataSource;
private final String clusterNodeId;
private final Map<String, MutableLock> locks;
private final DbLockManager dbLockManager;
public static final String SELECT_ABORT_STATEMENT = "SELECT * FROM " + TABLE_NAME_LOCK + " WHERE status='ABORT' AND lockOwner=?";
public LockThreadInterrupter(final DataSource dataSource, final String clusterNodeId, final Map<String, MutableLock> locks) {
public LockThreadInterrupter(final DataSource dataSource, final String clusterNodeId, final DbLockManager dbLockManager) {
this.dataSource = dataSource;
this.clusterNodeId = clusterNodeId;
this.locks = locks;
this.dbLockManager = dbLockManager;
}
public void run() {
Connection connection = null;
boolean originalAutoCommit = false;
......@@ -69,7 +70,7 @@ public class LockThreadInterrupter implements Runnable {
continue;
}
boolean lockThreadForAbortFound = false;
for (MutableLock lock : locks.values()) {
for (MutableLock lock : dbLockManager.getLocalLocks().values()) {
Thread thread = lock.getThread().get();
if (thread == null || !thread.isAlive()) {
// ignore since will be picked up by org.onehippo.services.lock.AbstractLockManager.UnlockStoppedThreadJanitor
......
......@@ -55,11 +55,11 @@ public class MemoryLockManager extends AbstractLockManager {
@Override
protected synchronized boolean containsLock(final String key) throws LockException {
return localLocks.containsKey(key);
return getLocalLocks().containsKey(key);
}
@Override
protected synchronized List<Lock> retrieveLocks() throws LockException {
return new ArrayList<>(localLocks.values());
return new ArrayList<>(getLocalLocks().values());
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment