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 d9c77150 authored by Ard Schrijvers's avatar Ard Schrijvers

REPO-1671 [Backport 4.2] make sure to synchronize on the jcr session to avoid thread safety issues

If we synchronize on the monitor object, there is room for concurrent access on the jcr session
because code that created the lock can call for example #isLive which can work on the
same jcr session as the background executor job does that refreshes the lock.

Also made sure that if #setTimeout fails with a repository exception, that this exception
is propagated and results in a lock exception. Before this exception would only be logged,
and situations were seen that the background thread that refreshes the lock would log
a repository exception in #setTimeout every 52 seconds (default 1 minute minus 8 second delay)
were it should abandon the refreshing completely.

In case of an exception, always log the stacktrace and not just in debug mode

(cherry picked from commit 2aa0536f)
parent 68aa3f3f
/*
* Copyright 2015 Hippo B.V. (http://www.onehippo.com)
* Copyright 2015-2017 Hippo B.V. (http://www.onehippo.com)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
......@@ -92,8 +92,7 @@ public class LockManagerDecorator extends org.hippoecm.repository.decorating.Loc
return new LockDecorator(super.getLock(absPath));
}
private void setTimeout(final Lock lock, final long timeoutHint) {
try {
private void setTimeout(final Lock lock, final long timeoutHint) throws RepositoryException {
final Node lockNode = lock.getNode();
if (timeoutHint != Long.MAX_VALUE) {
lockNode.addMixin(NT_LOCKABLE);
......@@ -107,25 +106,18 @@ public class LockManagerDecorator extends org.hippoecm.repository.decorating.Loc
}
}
lockNode.getSession().save();
} catch (RepositoryException e) {
if (log.isDebugEnabled()) {
log.error("Failed to set hippo:timeout on lock", e);
} else {
log.error("Failed to set hippo:timeout on lock: {}", e.toString());
}
}
}
public class LockDecorator implements HippoLock {
private Lock lock;
private volatile ScheduledFuture future;
private final Object monitor = this; // guards future
private final long timeout;
private LockDecorator(final Lock lock, final long timeout) {
this.lock = lock;
this.timeout = timeout;
}
private LockDecorator(final Lock lock) throws RepositoryException {
......@@ -135,49 +127,67 @@ public class LockManagerDecorator extends org.hippoecm.repository.decorating.Loc
@Override
public String getLockOwner() {
synchronized (session) {
return lock.getLockOwner();
}
}
@Override
public boolean isDeep() {
synchronized (session) {
return lock.isDeep();
}
}
@Override
public Node getNode() {
synchronized (session) {
return lock.getNode();
}
}
@Override
public String getLockToken() {
synchronized (session) {
return lock.getLockToken();
}
}
@Override
public long getSecondsRemaining() throws RepositoryException {
synchronized (session) {
return lock.getSecondsRemaining();
}
}
@Override
public boolean isLive() throws RepositoryException {
synchronized (session) {
return lock.isLive();
}
}
@Override
public boolean isSessionScoped() {
synchronized (session) {
return lock.isSessionScoped();
}
}
@Override
public boolean isLockOwningSession() {
synchronized (session) {
return lock.isLockOwningSession();
}
}
@Override
public void refresh() throws LockException, RepositoryException {
synchronized (session) {
lock.refresh();
setTimeout(lock, getSecondsRemaining());
}
}
@Override
public void startKeepAlive() throws RepositoryException {
......@@ -199,7 +209,7 @@ public class LockManagerDecorator extends org.hippoecm.repository.decorating.Loc
future = executor.schedule(new Runnable() {
@Override
public void run() {
synchronized (monitor) {
synchronized (session) {
if (future.isCancelled()) {
return;
}
......@@ -219,18 +229,10 @@ public class LockManagerDecorator extends org.hippoecm.repository.decorating.Loc
setTimeout(lock, timeout);
success = true;
} catch (RepositoryException e1) {
if (log.isDebugEnabled()) {
log.error("Failed to refresh lock", e1);
} else {
log.error("Failed to refresh lock: " + e1);
}
}
} catch (RepositoryException e) {
if (log.isDebugEnabled()) {
log.error("Failed to refresh lock", e);
} else {
log.error("Failed to refresh lock: " + e);
}
}
if (success) {
try {
......
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