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 2010/09/15 22:48:05 UTC

svn commit: r997496 - in /jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core: DefaultSecurityManager.java RepositoryImpl.java UserPerWorkspaceSecurityManager.java

Author: jukka
Date: Wed Sep 15 20:48:05 2010
New Revision: 997496

URL: http://svn.apache.org/viewvc?rev=997496&view=rev
Log:
JCR-2749: Closing a session twice shouldn't write a warning in the log

Use an explicit "active" flag instead of the activeSessions set to mark workspaces that shouldn't get disposed by the workspace janitor. This way the repository won't prematurely close the system session during shutdown.

Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/DefaultSecurityManager.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/RepositoryImpl.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/UserPerWorkspaceSecurityManager.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/DefaultSecurityManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/DefaultSecurityManager.java?rev=997496&r1=997495&r2=997496&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/DefaultSecurityManager.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/DefaultSecurityManager.java Wed Sep 15 20:48:05 2010
@@ -567,16 +567,21 @@ public class DefaultSecurityManager impl
         checkInitialized();
         AccessControlProvider provider = acProviders.get(workspaceName);
         if (provider == null || !provider.isLive()) {
-            SystemSession systemSession = repository.getSystemSession(workspaceName);
-            // mark this session as 'active' so the workspace does not get disposed
-            // by the workspace-janitor until the garbage collector is done
-            // TODO: review again... this workaround is now used in several places.
-            repository.onSessionCreated(systemSession);
+            // mark this workspace as 'active' so the workspace does not
+            // get disposed by the workspace-janitor
+            // TODO: There should be a cleaner way to do this.
+            repository.markWorkspaceActive(workspaceName);
+
+            WorkspaceSecurityConfig secConf = null;
+            WorkspaceConfig conf =
+                repository.getConfig().getWorkspaceConfig(workspaceName);
+            if (conf != null) {
+                secConf = conf.getSecurityConfig();
+            }
 
-            WorkspaceConfig conf = repository.getConfig().getWorkspaceConfig(workspaceName);
-            WorkspaceSecurityConfig secConf = (conf == null) ?  null : conf.getSecurityConfig();
+            provider = acProviderFactory.createProvider(
+                    repository.getSystemSession(workspaceName), secConf);
             synchronized (acProviders) {
-                provider = acProviderFactory.createProvider(systemSession, secConf);
                 acProviders.put(workspaceName, provider);
             }
         }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/RepositoryImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/RepositoryImpl.java?rev=997496&r1=997495&r2=997496&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/RepositoryImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/RepositoryImpl.java Wed Sep 15 20:48:05 2010
@@ -472,16 +472,17 @@ public class RepositoryImpl extends Abst
         if (smc != null && smc.getWorkspaceName() != null) {
             workspaceName = smc.getWorkspaceName();
         }
-        SystemSession securitySession = getSystemSession(workspaceName);
-        // mark system session as 'active' for that the system workspace does
-        // not get disposed by workspace-janitor
-        onSessionCreated(securitySession);
+
+        // mark the workspace as 'active' for that it does not get disposed
+        // by the workspace-janitor
+        // TODO: There should be a cleaner way to do this
+        markWorkspaceActive(workspaceName);
 
         // FIXME: Note that this call must be done *after* the security
         // manager has been added to the repository context, since the
         // initialisation code may invoke code that depends on the presence
         // of a security manager. It would be better if this was not the case.
-        securityMgr.init(this, securitySession);
+        securityMgr.init(this, getSystemSession(workspaceName));
     }
 
     /**
@@ -906,6 +907,21 @@ public class RepositoryImpl extends Abst
     }
 
     /**
+     * Marks the specified workspace as "active", so that the workspace
+     * janitor won't attempt to dispose it. This is used by features like
+     * security managers and the data store garbage collector to prevent
+     * workspaces from disappearing from below them.
+     * <p>
+     * FIXME: There should be a cleaner way to do this.
+     *
+     * @param workspaceName workspace name
+     * @throws RepositoryException if the workspace can not be accessed
+     */
+    void markWorkspaceActive(String workspaceName) throws RepositoryException {
+        getWorkspaceInfo(workspaceName).setActive(true);
+    }
+
+    /**
      * Creates a new repository session on the specified workspace for the
      * <b><i>authenticated</i></b> subject of the given login context and
      * adds it to the <i>active</i> sessions.
@@ -1381,15 +1397,19 @@ public class RepositoryImpl extends Abst
         for (int i = 0; i < wspNames.length; i++) {
             String wspName = wspNames[i];
             WorkspaceInfo wspInfo = getWorkspaceInfo(wspName);
+
             // this will initialize the workspace if required
             SessionImpl systemSession =
                 SystemSession.create(context, wspInfo.getConfig());
-            // mark this session as 'active' so the workspace does not get disposed
-            // by the workspace-janitor until the garbage collector is done
-            onSessionCreated(systemSession);
-            // the workspace could be disposed again, so re-initialize if required
-            // afterwards it will not be disposed because a session is registered
+
+            // mark the workspace as 'active' so it does not get disposed by
+            // the workspace-janitor until the garbage collector is done
+            wspInfo.setActive(true);
+
+            // the workspace could be disposed, so re-initialize if required
+            // afterwards it will not be disposed because it was marked active
             wspInfo.initialize();
+
             sessions[i] = systemSession;
             pm = wspInfo.getPersistenceManager();
             pmList.add(pm);
@@ -1641,6 +1661,12 @@ public class RepositoryImpl extends Abst
         private boolean initialized;
 
         /**
+         * Flag used to mark this as an "active" workspace that should not
+         * get automatically disposed by the workspace janitor.
+         */
+        private boolean active;
+
+        /**
          * lock that guards the initialization of this instance
          */
         private final ReadWriteLock initLock =
@@ -1738,6 +1764,14 @@ public class RepositoryImpl extends Abst
             return ret;
         }
 
+        public boolean isActive() {
+            return active;
+        }
+
+        public void setActive(boolean active) {
+            this.active = active;
+        }
+
         /**
          * Returns the workspace file system.
          *
@@ -2062,7 +2096,7 @@ public class RepositoryImpl extends Abst
                 return;
             }
             try {
-                if (!initialized) {
+                if (!initialized || active) {
                     return;
                 }
                 long currentTS = System.currentTimeMillis();
@@ -2103,6 +2137,7 @@ public class RepositoryImpl extends Abst
                 // reset idle timestamp
                 idleTimestamp = 0;
 
+                active = false;
                 initialized = false;
                 log.info("workspace '" + getName() + "' has been shutdown");
             } finally {

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/UserPerWorkspaceSecurityManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/UserPerWorkspaceSecurityManager.java?rev=997496&r1=997495&r2=997496&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/UserPerWorkspaceSecurityManager.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/UserPerWorkspaceSecurityManager.java Wed Sep 15 20:48:05 2010
@@ -84,7 +84,7 @@ public class UserPerWorkspaceSecurityMan
                     RepositoryImpl repo = (RepositoryImpl) getRepository();
                     systemSession = repo.getSystemSession(wspName);
                     // TODO: review again... this workaround is used in several places.
-                    repo.onSessionCreated(systemSession);
+                    repo.markWorkspaceActive(wspName);
                 }
 
                 PrincipalProvider defaultPP = new DefaultPrincipalProvider(systemSession, (UserManagerImpl) getUserManager(systemSession));