From c524b60012e4c20f7d7796098d8fdacc6e09b531 Mon Sep 17 00:00:00 2001 From: Jukka Zitting Date: Wed, 6 Apr 2011 14:18:58 +0200 Subject: [PATCH] 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. --- .../org/apache/jackrabbit/core/SystemSession.java | 21 +++ .../security/authorization/acl/ACLEventSieve.java | 191 ++++++++++++++++++++ .../security/authorization/acl/EntryCollector.java | 121 +++---------- 3 files changed, 237 insertions(+), 96 deletions(-) create mode 100644 jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLEventSieve.java diff --git a/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SystemSession.java b/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SystemSession.java index d6f27c7..b803476 100644 --- a/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SystemSession.java +++ b/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SystemSession.java @@ -122,6 +122,27 @@ class SystemSession extends SessionImpl { return false; } + /** + * Creates and returns a new system session for the + * given workspace. + * + * @param workspaceName workspace name, + * or null 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. diff --git a/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLEventSieve.java b/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLEventSieve.java new file mode 100644 index 0000000..23364df --- /dev/null +++ b/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLEventSieve.java @@ -0,0 +1,191 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.jackrabbit.core.security.authorization.acl; + +import java.util.HashMap; +import java.util.Map; + +import javax.jcr.Node; +import javax.jcr.RepositoryException; +import javax.jcr.Session; +import javax.jcr.observation.Event; +import javax.jcr.observation.EventIterator; + +import org.apache.jackrabbit.core.NodeImpl; +import org.apache.jackrabbit.core.id.NodeId; +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; + +/** + * Package-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} class. + */ +class ACLEventSieve { + + /** Logger instance */ + private static final Logger log = + LoggerFactory.getLogger(ACLEventSieve.class); + + /** 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 modMap = + new HashMap(); + + 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: + siftNodeAdded(event.getIdentifier()); + break; + case Event.NODE_REMOVED: + siftNodeRemoved(event.getPath()); + break; + case Event.PROPERTY_CHANGED: + 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); + } + } + } + + /** + * Returns the access control modifications collected from + * related observation events. + * + * @return access control modifications + */ + public AccessControlModifications getModifications() { + return new AccessControlModifications(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 diff --git a/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/EntryCollector.java b/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/EntryCollector.java index 893cd2e..790ee59 100644 --- a/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/EntryCollector.java +++ b/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/EntryCollector.java @@ -22,12 +22,11 @@ import org.apache.jackrabbit.core.id.NodeId; 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.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 +60,6 @@ public class EntryCollector extends AccessControlObserver implements AccessContr * The root id. */ protected final NodeId rootID; - - /** - * Standard JCR name form of the {@link #N_POLICY} constant. - */ - private final String repPolicyName; /** * @@ -77,8 +71,6 @@ public class EntryCollector extends AccessControlObserver implements AccessContr 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 +193,39 @@ public class EntryCollector extends AccessControlObserver implements AccessContr 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 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 modMap = new HashMap(); - - // 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(); - - switch (ev.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. */ - 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)."); - } - 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. */ - break; - default: - // illegal event-type: should never occur. ignore + // 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 mods = + sieve.getModifications(); + if (!mods.getNodeIdentifiers().isEmpty()) { + notifyListeners(mods); } - } catch (RepositoryException e) { - // should not get here - log.error("Internal error: ", e); + } finally { + session.logout(); } - } - - if (!modMap.isEmpty()) { - // notify listeners and eventually clean up internal caches. - notifyListeners(new AccessControlModifications(modMap)); + } catch (RepositoryException e) { + log.error("Failed to process access control modifications", e); } } + } \ No newline at end of file -- 1.7.4