You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by ju...@apache.org on 2011/04/06 14:59:44 UTC
svn commit: r1089436 - in
/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core:
SystemSession.java security/authorization/acl/EntryCollector.java
Author: jukka
Date: Wed Apr 6 12:59:44 2011
New Revision: 1089436
URL: http://svn.apache.org/viewvc?rev=1089436&view=rev
Log:
JCR-2890: Deadlock in acl.EntryCollector / ItemManager
Use a fresh new session in EntryCollector.onEvents() for sifting through the observed changes. This prevents a deadlock with other threads that are concurrently using the systemSession instance.
Modified:
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SystemSession.java
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/EntryCollector.java
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SystemSession.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SystemSession.java?rev=1089436&r1=1089435&r2=1089436&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SystemSession.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SystemSession.java Wed Apr 6 12:59:44 2011
@@ -122,6 +122,27 @@ class SystemSession extends SessionImpl
return false;
}
+ /**
+ * Creates and returns a new <em>system</em> session for the
+ * given workspace.
+ *
+ * @param workspaceName workspace name,
+ * or <code>null</code> for the default workspace
+ */
+ @Override
+ public SessionImpl createSession(String workspaceName)
+ throws RepositoryException {
+ if (workspaceName == null) {
+ WorkspaceManager wm = repositoryContext.getWorkspaceManager();
+ workspaceName = wm.getDefaultWorkspaceName();
+ }
+
+ RepositoryImpl repository = repositoryContext.getRepository();
+ WorkspaceConfig wspConfig =
+ repository.getWorkspaceInfo(workspaceName).getConfig();
+ return create(repositoryContext, wspConfig);
+ }
+
//--------------------------------------------------------< inner classes >
/**
* An access manager that grants access to everything.
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/EntryCollector.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/EntryCollector.java?rev=1089436&r1=1089435&r2=1089436&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/EntryCollector.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/EntryCollector.java Wed Apr 6 12:59:44 2011
@@ -22,12 +22,14 @@ import org.apache.jackrabbit.core.id.Nod
import org.apache.jackrabbit.core.security.authorization.AccessControlConstants;
import org.apache.jackrabbit.core.security.authorization.AccessControlModifications;
import org.apache.jackrabbit.core.security.authorization.AccessControlObserver;
+import org.apache.jackrabbit.spi.Name;
import org.apache.jackrabbit.util.Text;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import javax.jcr.Node;
import javax.jcr.RepositoryException;
+import javax.jcr.Session;
import javax.jcr.observation.Event;
import javax.jcr.observation.EventIterator;
import javax.jcr.observation.ObservationManager;
@@ -61,11 +63,6 @@ public class EntryCollector extends Acce
* The root id.
*/
protected final NodeId rootID;
-
- /**
- * Standard JCR name form of the {@link #N_POLICY} constant.
- */
- private final String repPolicyName;
/**
*
@@ -77,8 +74,6 @@ public class EntryCollector extends Acce
this.systemSession = systemSession;
this.rootID = rootID;
- repPolicyName = systemSession.getJCRName(N_POLICY);
-
ObservationManager observationMgr = systemSession.getWorkspace().getObservationManager();
/*
Make sure the collector and all subscribed listeners are informed upon
@@ -201,102 +196,191 @@ public class EntryCollector extends Acce
return ((NodeImpl) systemSession.getItemManager().getItem(nodeId));
}
- //------------------------------------------------------------< private >---
-
- private static NodeId accessControlledIdFromAclNode(Node aclNode) throws RepositoryException {
- return ((NodeImpl) aclNode.getParent()).getNodeId();
- }
-
- private static NodeId accessControlledIdFromAceNode(Node aceNode) throws RepositoryException {
- return ((NodeImpl) aceNode.getParent().getParent()).getNodeId();
- }
-
- private static void addModification(NodeId accessControllNodeId, int modType,
- Map<NodeId,Integer> modMap) {
- if (modMap.containsKey(accessControllNodeId)) {
- // update modMap
- modMap.put(accessControllNodeId, modType | modMap.get(accessControllNodeId));
- } else {
- modMap.put(accessControllNodeId, modType);
- }
- }
-
//------------------------------------------------------< EventListener >---
+
/**
- * Collect is of access controlled nodes that are effected by access control
- * modification together with the corresponding modification type and
- * finally inform listeners about the modifications.
+ * Collects access controlled nodes that are effected by access control
+ * changes together with the corresponding modification types, and
+ * notifies access control listeners about the modifications.
*
* @param events
*/
public void onEvent(EventIterator events) {
- /* map of access-controlled nodeId to type of ac modification */
- Map<NodeId,Integer> modMap = new HashMap<NodeId,Integer>();
-
- // collect the ids of all access controlled nodes that have been affected
- // by the events and thus need their cache entries updated or cleared.
- while (events.hasNext()) {
+ try {
+ // JCR-2890: We need to use a fresh new session here to avoid
+ // deadlocks caused by concurrent threads possibly using the
+ // systemSession instance for other purposes.
+ String workspaceName = systemSession.getWorkspace().getName();
+ Session session = systemSession.createSession(workspaceName);
try {
- Event ev = events.nextEvent();
- String identifier = ev.getIdentifier();
- String path = ev.getPath();
+ // Sift through the events to find access control modifications
+ ACLEventSieve sieve = new ACLEventSieve(session);
+ sieve.siftEvents(events);
+
+ // Notify listeners and eventually clean up internal caches
+ AccessControlModifications<NodeId> mods =
+ sieve.getModifications();
+ if (!mods.getNodeIdentifiers().isEmpty()) {
+ notifyListeners(mods);
+ }
+ } finally {
+ session.logout();
+ }
+ } catch (RepositoryException e) {
+ log.error("Failed to process access control modifications", e);
+ }
+ }
- switch (ev.getType()) {
+ /**
+ * Private utility class for sifting through observation events on
+ * ACL, ACE and Policy nodes to find out the nodes whose access controls
+ * have changed. Used by the {@link EntryCollector#onEvent(EventIterator)}
+ * method.
+ */
+ private static class ACLEventSieve {
+
+ /** Session with system privileges. */
+ private final Session session;
+
+ /**
+ * Standard JCR name form of the
+ * {@link AccessControlConstants#N_POLICY} constant.
+ */
+ private final String repPolicyName;
+
+ /**
+ * Map of access-controlled nodeId to type of access control modification.
+ */
+ private final Map<NodeId, Integer> modMap =
+ new HashMap<NodeId,Integer>();
+
+ public ACLEventSieve(Session session) throws RepositoryException {
+ this.session = session;
+ Name repPolicy = AccessControlConstants.N_POLICY;
+ this.repPolicyName =
+ session.getNamespacePrefix(repPolicy.getNamespaceURI())
+ + ":" + repPolicy.getLocalName();
+ }
+
+ /**
+ * Collects the identifiers of all access controlled nodes that have
+ * been affected by the events, and thus need their cache entries
+ * updated or cleared.
+ *
+ * @param events access control modification events
+ */
+ public void siftEvents(EventIterator events) {
+ while (events.hasNext()) {
+ Event event = events.nextEvent();
+ try {
+ switch (event.getType()) {
case Event.NODE_ADDED:
- NodeImpl n = (NodeImpl) systemSession.getNodeByIdentifier(identifier);
- if (n.isNodeType(NT_REP_ACL)) {
- // a new ACL was added -> use the added node to update
- // the cache.
- addModification(accessControlledIdFromAclNode(n), POLICY_ADDED, modMap);
- } else if (n.isNodeType(NT_REP_ACE)) {
- // a new ACE was added -> use the parent node (acl)
- // to update the cache.
- addModification(accessControlledIdFromAceNode(n), POLICY_MODIFIED, modMap);
- } /* else: some other node added below an access controlled
- parent node -> not interested. */
+ siftNodeAdded(event.getIdentifier());
break;
case Event.NODE_REMOVED:
- String parentPath = Text.getRelativeParent(path, 1);
- if (systemSession.nodeExists(parentPath)) {
- NodeImpl parent = (NodeImpl) systemSession.getNode(parentPath);
- if (repPolicyName.equals(Text.getName(path))){
- // the complete acl was removed -> clear cache entry
- addModification(parent.getNodeId(), POLICY_REMOVED, modMap);
- } else if (parent.isNodeType(NT_REP_ACL)) {
- // an ace was removed -> refresh cache for the
- // containing access control list upon next access
- addModification(accessControlledIdFromAclNode(parent), POLICY_MODIFIED, modMap);
- } /* else:
- a) some other child node of an access controlled
- node -> not interested.
- b) a child node of an ACE. not relevant for this
- implementation -> ignore
- */
- } else {
- log.debug("Cannot process NODE_REMOVED event. Parent " + parentPath + " doesn't exist (anymore).");
- }
+ siftNodeRemoved(event.getPath());
break;
case Event.PROPERTY_CHANGED:
- // test if the changed prop belongs to an ACE
- NodeImpl parent = (NodeImpl) systemSession.getNodeByIdentifier(identifier);
- if (parent.isNodeType(NT_REP_ACE)) {
- addModification(accessControlledIdFromAceNode(parent), POLICY_MODIFIED, modMap);
- } /* some other property below an access controlled node
- changed -> not interested. (NOTE: rep:ACL doesn't
- define any properties. */
+ siftPropertyChanged(event.getIdentifier());
break;
default:
// illegal event-type: should never occur. ignore
+ }
+ } catch (RepositoryException e) {
+ // should not get here
+ log.error("Failed to process ACL event: " + event, e);
}
- } catch (RepositoryException e) {
- // should not get here
- log.error("Internal error: ", e);
}
}
- if (!modMap.isEmpty()) {
- // notify listeners and eventually clean up internal caches.
- notifyListeners(new AccessControlModifications<NodeId>(modMap));
+ /**
+ * Returns the access control modifications collected from
+ * related observation events.
+ *
+ * @return access control modifications
+ */
+ public AccessControlModifications<NodeId> getModifications() {
+ return new AccessControlModifications<NodeId>(modMap);
+ }
+
+ private void siftNodeAdded(String identifier)
+ throws RepositoryException {
+ NodeImpl n = (NodeImpl) session.getNodeByIdentifier(identifier);
+ if (n.isNodeType(EntryCollector.NT_REP_ACL)) {
+ // a new ACL was added -> use the added node to update
+ // the cache.
+ addModification(
+ accessControlledIdFromAclNode(n),
+ AccessControlObserver.POLICY_ADDED);
+ } else if (n.isNodeType(EntryCollector.NT_REP_ACE)) {
+ // a new ACE was added -> use the parent node (acl)
+ // to update the cache.
+ addModification(
+ accessControlledIdFromAceNode(n),
+ AccessControlObserver.POLICY_MODIFIED);
+ } /* else: some other node added below an access controlled
+ parent node -> not interested. */
+ }
+
+ private void siftNodeRemoved(String path) throws RepositoryException {
+ String parentPath = Text.getRelativeParent(path, 1);
+ if (session.nodeExists(parentPath)) {
+ NodeImpl parent = (NodeImpl) session.getNode(parentPath);
+ if (repPolicyName.equals(Text.getName(path))){
+ // the complete ACL was removed -> clear cache entry
+ addModification(
+ parent.getNodeId(),
+ AccessControlObserver.POLICY_REMOVED);
+ } else if (parent.isNodeType(EntryCollector.NT_REP_ACL)) {
+ // an ace was removed -> refresh cache for the
+ // containing access control list upon next access
+ addModification(
+ accessControlledIdFromAclNode(parent),
+ AccessControlObserver.POLICY_MODIFIED);
+ } /* else:
+ a) some other child node of an access controlled
+ node -> not interested.
+ b) a child node of an ACE. not relevant for this
+ implementation -> ignore
+ */
+ } else {
+ log.debug("Cannot process NODE_REMOVED event."
+ + " Parent {} doesn't exist (anymore).",
+ parentPath);
+ }
+ }
+
+ private void siftPropertyChanged(String identifier)
+ throws RepositoryException {
+ // test if the changed prop belongs to an ACE
+ NodeImpl parent = (NodeImpl) session.getNodeByIdentifier(identifier);
+ if (parent.isNodeType(EntryCollector.NT_REP_ACE)) {
+ addModification(
+ accessControlledIdFromAceNode(parent),
+ AccessControlObserver.POLICY_MODIFIED);
+ } /* some other property below an access controlled node
+ changed -> not interested. (NOTE: rep:ACL doesn't
+ define any properties. */
}
+
+ private NodeId accessControlledIdFromAclNode(Node aclNode)
+ throws RepositoryException {
+ return ((NodeImpl) aclNode.getParent()).getNodeId();
+ }
+
+ private NodeId accessControlledIdFromAceNode(Node aceNode)
+ throws RepositoryException {
+ return accessControlledIdFromAclNode(aceNode.getParent());
+ }
+
+ private void addModification(NodeId accessControllNodeId, int modType) {
+ if (modMap.containsKey(accessControllNodeId)) {
+ // update modMap
+ modType |= modMap.get(accessControllNodeId);
+ }
+ modMap.put(accessControllNodeId, modType);
+ }
+
}
+
}
\ No newline at end of file