jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ju...@apache.org
Subject svn commit: r1089436 - in /jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core: SystemSession.java security/authorization/acl/EntryCollector.java
Date Wed, 06 Apr 2011 12:59:44 GMT
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



Mime
View raw message