Commit dfc32cc5 authored by Niels Charlier's avatar Niels Charlier
Browse files

GEOS-9096 jdbcconfig/hazelcast thread safety fixes

parent 41a454df
......@@ -17,6 +17,8 @@ import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.UUID;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.Nullable;
import javax.annotation.ParametersAreNonnullByDefault;
import org.geoserver.catalog.Catalog;
......@@ -40,6 +42,7 @@ import org.geoserver.catalog.util.CloseableIterator;
import org.geoserver.jdbcconfig.internal.ConfigDatabase;
import org.geoserver.ows.util.OwsUtils;
import org.geotools.util.Utilities;
import org.geotools.util.logging.Logging;
import org.opengis.filter.Filter;
import org.opengis.filter.sort.SortBy;
import org.springframework.util.Assert;
......@@ -48,6 +51,8 @@ import org.springframework.util.Assert;
@ParametersAreNonnullByDefault
public class JDBCCatalogFacade implements CatalogFacade {
public static final Logger LOGGER = Logging.getLogger(JDBCCatalogFacade.class);
private final ConfigDatabase db;
public JDBCCatalogFacade(final ConfigDatabase db) {
......@@ -873,9 +878,12 @@ public class JDBCCatalogFacade implements CatalogFacade {
protected <T extends CatalogInfo> T commitProxy(T object) {
// get the real object
T real = db.save(object);
return real;
try {
return db.save(object);
} catch (Exception e) {
LOGGER.log(Level.SEVERE, "Failed to save object " + object.getId(), e);
return null;
}
}
protected void afterSaved(
......
......@@ -49,7 +49,8 @@ import java.util.concurrent.Callable;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.locks.ReentrantLock;
import java.util.concurrent.Semaphore;
import java.util.concurrent.TimeUnit;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.Nullable;
......@@ -119,6 +120,8 @@ public class ConfigDatabase {
public static final Logger LOGGER = Logging.getLogger(ConfigDatabase.class);
private static final int LOCK_TIMEOUT_SECONDS = 60;
private Dialect dialect;
private DataSource dataSource;
......@@ -145,7 +148,7 @@ public class ConfigDatabase {
private ConfigClearingListener configListener;
private ConcurrentMap<String, ReentrantLock> locks;
private ConcurrentMap<String, Semaphore> locks;
/** Protected default constructor needed by spring-jdbc instrumentation */
protected ConfigDatabase() {
......@@ -1003,20 +1006,16 @@ public class ConfigDatabase {
valueLoader = new ConfigLoader(id);
}
ReentrantLock lock = locks.get(id);
if (lock == null) {
lock = new ReentrantLock();
locks.put(id, lock);
}
Semaphore lock = locks.computeIfAbsent(id, x -> new Semaphore(1));
info = cache.getIfPresent(id);
if (info == null) {
// we try the write lock
if (!lock.isHeldByCurrentThread() && lock.tryLock()) {
if (lock.tryAcquire()) {
try {
info = cache.get(id, valueLoader);
} finally {
lock.unlock();
lock.release();
}
}
}
......@@ -1537,16 +1536,21 @@ public class ConfigDatabase {
}
private void acquireWriteLock(String id) {
ReentrantLock lock = locks.get(id);
if (lock == null) {
lock = new ReentrantLock();
locks.put(id, lock);
Semaphore lock = locks.computeIfAbsent(id, x -> new Semaphore(1));
try {
if (!lock.tryAcquire(LOCK_TIMEOUT_SECONDS, TimeUnit.SECONDS)) {
LOGGER.severe(
"Time-out waiting for lock on "
+ id
+ ", assuming it was abandoned and moving on. This shouldn't happen!");
}
} catch (InterruptedException e) {
}
lock.lock();
}
private void releaseWriteLock(String id) {
locks.get(id).unlock();
locks.get(id).release();
}
/** Listens to catalog events clearing cache entires when resources are modified. */
......
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