Author: kkolinko
Date: Wed Jul  9 02:31:47 2014
New Revision: 1608993

URL: http://svn.apache.org/r1608993
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56653
Fix concurrency issue with lists of contexts in Mapper when stopping Contexts.

The ContextList fields are made final.
Instead of sharing the same ContextList object instance among Hosts,
the ContextList is made an immutable object and is updated via 
updateContextList() method.

Modified:
    tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/mapper/Mapper.java
    tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml

Modified: 
tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/mapper/Mapper.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/mapper/Mapper.java?rev=1608993&r1=1608992&r2=1608993&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/mapper/Mapper.java 
(original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/mapper/Mapper.java 
Wed Jul  9 02:31:47 2014
@@ -192,6 +192,16 @@ public final class Mapper {
 
     }
 
+    /**
+     * Replace {@link Host#contextList} field in <code>realHost</code> and
+     * all its aliases with a new value.
+     */
+    private void updateContextList(Host realHost, ContextList newContextList) {
+        realHost.contextList = newContextList;
+        for (Host alias : realHost.getAliases()) {
+            alias.contextList = newContextList;
+        }
+    }
 
     /**
      * Set context, used for wrapper mapping (request dispatcher).
@@ -245,17 +255,14 @@ public final class Mapper {
             newContextVersion.welcomeResources = welcomeResources;
             newContextVersion.resources = resources;
 
-            Context[] contexts = mappedHost.contextList.contexts;
-            // Update nesting
-            if (slashCount > mappedHost.contextList.nesting) {
-                mappedHost.contextList.nesting = slashCount;
-            }
-            Context mappedContext = exactFind(contexts, path);
+            ContextList contextList = mappedHost.contextList;
+            Context mappedContext = exactFind(contextList.contexts, path);
             if (mappedContext == null) {
                 mappedContext = new Context(path, newContextVersion);
-                Context[] newContexts = new Context[contexts.length + 1];
-                if (insertMap(contexts, newContexts, mappedContext)) {
-                    mappedHost.contextList.contexts = newContexts;
+                ContextList newContextList = contextList.addContext(
+                        mappedContext, slashCount);
+                if (newContextList != null) {
+                    updateContextList(mappedHost, newContextList);
                 }
             } else {
                 ContextVersion[] contextVersions = mappedContext.versions;
@@ -284,8 +291,8 @@ public final class Mapper {
             return;
         }
         synchronized (host) {
-            Context[] contexts = host.contextList.contexts;
-            Context context = exactFind(contexts, path);
+            ContextList contextList = host.contextList;
+            Context context = exactFind(contextList.contexts, path);
             if (context == null) {
                 return;
             }
@@ -296,17 +303,9 @@ public final class Mapper {
             if (removeMap(contextVersions, newContextVersions, version)) {
                 if (context.versions.length == 0) {
                     // Remove the context
-                    Context[] newContexts = new Context[contexts.length -1];
-                    if (removeMap(contexts, newContexts, path)) {
-                        host.contextList.contexts = newContexts;
-                        // Recalculate nesting
-                        host.contextList.nesting = 0;
-                        for (int i = 0; i < newContexts.length; i++) {
-                            int slashCount = slashCount(newContexts[i].name);
-                            if (slashCount > host.contextList.nesting) {
-                                host.contextList.nesting = slashCount;
-                            }
-                        }
+                    ContextList newContextList = 
contextList.removeContext(path);
+                    if (newContextList != null) {
+                        updateContextList(host, newContextList);
                     }
                 } else {
                     context.versions = newContextVersions;
@@ -1463,7 +1462,7 @@ public final class Mapper {
 
     protected static final class Host extends MapElement {
 
-        public ContextList contextList;
+        public volatile ContextList contextList;
 
         /**
          * Link to the "real" Host, shared by all aliases.
@@ -1532,9 +1531,38 @@ public final class Mapper {
 
     protected static final class ContextList {
 
-        public Context[] contexts = new Context[0];
-        public int nesting = 0;
+        public final Context[] contexts;
+        public final int nesting;
+
+        public ContextList() {
+            this(new Context[0], 0);
+        }
 
+        private ContextList(Context[] contexts, int nesting) {
+            this.contexts = contexts;
+            this.nesting = nesting;
+        }
+
+        public ContextList addContext(Context mappedContext, int slashCount) {
+            Context[] newContexts = new Context[contexts.length + 1];
+            if (insertMap(contexts, newContexts, mappedContext)) {
+                return new ContextList(newContexts, Math.max(nesting,
+                        slashCount));
+            }
+            return null;
+        }
+
+        public ContextList removeContext(String path) {
+            Context[] newContexts = new Context[contexts.length - 1];
+            if (removeMap(contexts, newContexts, path)) {
+                int newNesting = 0;
+                for (Context context : newContexts) {
+                    newNesting = Math.max(newNesting, 
slashCount(context.name));
+                }
+                return new ContextList(newContexts, newNesting);
+            }
+            return null;
+        }
     }
 
 

Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1608993&r1=1608992&r2=1608993&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Wed Jul  9 02:31:47 2014
@@ -113,6 +113,10 @@
         (markt/kkolinko)
       </scode>
       <fix>
+        <bug>56653</bug>: Fix concurrency issue with lists of contexts in
+        <code>Mapper</code> when stopping Contexts. (kkolinko)
+      </fix>
+      <fix>
         <bug>56657</bug>: When using parallel deployment, if the same session 
id
         matches different versions of a web application, prefer the latest
         version. Ensure that remapping selects the version that we expect.



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to