You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2006/06/10 18:45:01 UTC

svn commit: r413328 - in /tomcat/container/branches/tc4.1.x/catalina/src/share/org/apache/catalina: servlets/HTMLManagerServlet.java servlets/ManagerServlet.java startup/HostConfig.java

Author: markt
Date: Sat Jun 10 09:45:00 2006
New Revision: 413328

URL: http://svn.apache.org/viewvc?rev=413328&view=rev
Log:
Fix bug 28845 - a possible race condition when uploading a war via Manager or HTMLManager 

Also fix an issue with HostConfig that would only ever auto-deploy an application once, even if that application was subsequently removed and then re-copied into the appbase

Modified:
    tomcat/container/branches/tc4.1.x/catalina/src/share/org/apache/catalina/servlets/HTMLManagerServlet.java
    tomcat/container/branches/tc4.1.x/catalina/src/share/org/apache/catalina/servlets/ManagerServlet.java
    tomcat/container/branches/tc4.1.x/catalina/src/share/org/apache/catalina/startup/HostConfig.java

Modified: tomcat/container/branches/tc4.1.x/catalina/src/share/org/apache/catalina/servlets/HTMLManagerServlet.java
URL: http://svn.apache.org/viewvc/tomcat/container/branches/tc4.1.x/catalina/src/share/org/apache/catalina/servlets/HTMLManagerServlet.java?rev=413328&r1=413327&r2=413328&view=diff
==============================================================================
--- tomcat/container/branches/tc4.1.x/catalina/src/share/org/apache/catalina/servlets/HTMLManagerServlet.java (original)
+++ tomcat/container/branches/tc4.1.x/catalina/src/share/org/apache/catalina/servlets/HTMLManagerServlet.java Sat Jun 10 09:45:00 2006
@@ -164,100 +164,104 @@
         FileItem warUpload = null;
         File xmlFile = null;
         
-        try {
-            List items = upload.parseRequest(request);
-        
-            // Process the uploaded fields
-            Iterator iter = items.iterator();
-            while (iter.hasNext()) {
-                FileItem item = (FileItem) iter.next();
-        
-                if (!item.isFormField()) {
-                    if (item.getFieldName().equals("installWar") &&
-                        warUpload == null) {
-                        warUpload = item;
-                    } else {
-                        item.delete();
+        // There is a possible race condition here. If liveDeploy is true it
+        // means the deployer could start to deploy the app before we do it.
+        synchronized(getLock()) { 
+            try {
+                List items = upload.parseRequest(request);
+            
+                // Process the uploaded fields
+                Iterator iter = items.iterator();
+                while (iter.hasNext()) {
+                    FileItem item = (FileItem) iter.next();
+            
+                    if (!item.isFormField()) {
+                        if (item.getFieldName().equals("installWar") &&
+                            warUpload == null) {
+                            warUpload = item;
+                        } else {
+                            item.delete();
+                        }
                     }
                 }
-            }
-            while(true) {
-                if (warUpload == null) {
-                    writer.println(sm.getString
-                        ("htmlManagerServlet.installUploadNoFile"));
-                    break;
-                }
-                war = warUpload.getName();
-                if (!war.toLowerCase().endsWith(".war")) {
-                    writer.println(sm.getString
-                        ("htmlManagerServlet.installUploadNotWar",war));
-                    break;
-                }
-                // Get the filename if uploaded name includes a path
-                if (war.lastIndexOf('\\') >= 0) {
-                    war = war.substring(war.lastIndexOf('\\') + 1);
-                }
-                if (war.lastIndexOf('/') >= 0) {
-                    war = war.substring(war.lastIndexOf('/') + 1);
-                }
-                
-                String xmlName = war.substring(0,war.length()-4) + ".xml";
-                
-                // Identify the appBase of the owning Host of this Context
-                // (if any)
-                String appBase = null;
-                File appBaseDir = null;
-                appBase = ((Host) context.getParent()).getAppBase();
-                appBaseDir = new File(appBase);
-                if (!appBaseDir.isAbsolute()) {
-                    appBaseDir = new File(System.getProperty("catalina.base"),
-                                          appBase);
-                }
-                File file = new File(appBaseDir,war);
-                if (file.exists()) {
-                    writer.println(sm.getString
-                        ("htmlManagerServlet.installUploadWarExists",war));
+                while(true) {
+                    if (warUpload == null) {
+                        writer.println(sm.getString
+                            ("htmlManagerServlet.installUploadNoFile"));
+                        break;
+                    }
+                    war = warUpload.getName();
+                    if (!war.toLowerCase().endsWith(".war")) {
+                        writer.println(sm.getString
+                            ("htmlManagerServlet.installUploadNotWar",war));
+                        break;
+                    }
+                    // Get the filename if uploaded name includes a path
+                    if (war.lastIndexOf('\\') >= 0) {
+                        war = war.substring(war.lastIndexOf('\\') + 1);
+                    }
+                    if (war.lastIndexOf('/') >= 0) {
+                        war = war.substring(war.lastIndexOf('/') + 1);
+                    }
+                    
+                    String xmlName = war.substring(0,war.length()-4) + ".xml";
+                    
+                    // Identify the appBase of the owning Host of this Context
+                    // (if any)
+                    String appBase = null;
+                    File appBaseDir = null;
+                    appBase = ((Host) context.getParent()).getAppBase();
+                    appBaseDir = new File(appBase);
+                    if (!appBaseDir.isAbsolute()) {
+                        appBaseDir = new File(System.getProperty("catalina.base"),
+                                              appBase);
+                    }
+                    File file = new File(appBaseDir,war);
+                    if (file.exists()) {
+                        writer.println(sm.getString
+                            ("htmlManagerServlet.installUploadWarExists",war));
+                        break;
+                    }
+                    warUpload.write(file);
+                    try {
+                        URL url = file.toURL();
+                        war = url.toString();
+                        war = "jar:" + war + "!/";
+                    } catch(MalformedURLException e) {
+                        file.delete();
+                        throw e;
+                    }
+                    
+                    // Extract the context.xml file, if any
+                    xmlFile = new File(appBaseDir, xmlName);
+                    extractXml(file, xmlFile);
+                    
+                    uploadFailed = false;
+                    
                     break;
                 }
-                warUpload.write(file);
-                try {
-                    URL url = file.toURL();
-                    war = url.toString();
-                    war = "jar:" + war + "!/";
-                } catch(MalformedURLException e) {
-                    file.delete();
-                    throw e;
+            } catch(Exception e) {
+                String message = sm.getString
+                    ("htmlManagerServlet.installUploadFail", e.getMessage());
+                log(message, e);
+                writer.println(message);
+            } finally {
+                if (warUpload != null) {
+                    warUpload.delete();
                 }
-                
-                // Extract the context.xml file, if any
-                xmlFile = new File(appBaseDir, xmlName);
-                extractXml(file, xmlFile);
-                
-                uploadFailed = false;
-                
-                break;
+                warUpload = null;
+            }
+        
+            // Define the context.xml URL if present
+            String xmlURL = null;
+            if (xmlFile != null && xmlFile.exists()) {
+                xmlURL = new String("file:" + xmlFile.getAbsolutePath());
             }
-        } catch(Exception e) {
-            String message = sm.getString
-                ("htmlManagerServlet.installUploadFail", e.getMessage());
-            log(message, e);
-            writer.println(message);
-        } finally {
-            if (warUpload != null) {
-                warUpload.delete();
+    
+            // If there were no errors, install the WAR
+            if (!uploadFailed) {
+                install(writer, xmlURL, null, war);
             }
-            warUpload = null;
-        }
-
-        // Define the context.xml URL if present
-        String xmlURL = null;
-        if (xmlFile != null && xmlFile.exists()) {
-            xmlURL = new String("file:" + xmlFile.getAbsolutePath());
-        }
-
-        // If there were no errors, install the WAR
-        if (!uploadFailed) {
-            install(writer, xmlURL, null, war);
         }
 
         String message = stringWriter.toString();

Modified: tomcat/container/branches/tc4.1.x/catalina/src/share/org/apache/catalina/servlets/ManagerServlet.java
URL: http://svn.apache.org/viewvc/tomcat/container/branches/tc4.1.x/catalina/src/share/org/apache/catalina/servlets/ManagerServlet.java?rev=413328&r1=413327&r2=413328&view=diff
==============================================================================
--- tomcat/container/branches/tc4.1.x/catalina/src/share/org/apache/catalina/servlets/ManagerServlet.java (original)
+++ tomcat/container/branches/tc4.1.x/catalina/src/share/org/apache/catalina/servlets/ManagerServlet.java Sat Jun 10 09:45:00 2006
@@ -41,18 +41,24 @@
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+
+import org.apache.catalina.Container;
 import org.apache.catalina.ContainerServlet;
 import org.apache.catalina.Context;
 import org.apache.catalina.Deployer;
 import org.apache.catalina.Globals;
 import org.apache.catalina.Host;
+import org.apache.catalina.Lifecycle;
+import org.apache.catalina.LifecycleListener;
 import org.apache.catalina.Role;
 import org.apache.catalina.Server;
 import org.apache.catalina.ServerFactory;
 import org.apache.catalina.Session;
 import org.apache.catalina.UserDatabase;
 import org.apache.catalina.Wrapper;
+import org.apache.catalina.core.StandardHost;
 import org.apache.catalina.core.StandardServer;
+import org.apache.catalina.startup.HostConfig;
 import org.apache.catalina.util.RequestUtil;
 import org.apache.catalina.util.ServerInfo;
 import org.apache.catalina.util.StringManager;
@@ -528,32 +534,32 @@
             return;
         }
 
-        // FIXME  There is a race condition here. If liveDeploy is true it means
-        // the deployer "could" start deploy the app before we start doing it.
-        // This would need host synchronization here and in HostConfig
-
-        // Deploy this web application
-        try {
-            URL warURL =
-                new URL("jar:file:" + localWar.getAbsolutePath() + "!/");
-            URL xmlURL = null;
-            if (localXml.exists()) {
-                xmlURL = new URL("file:" + localXml.getAbsolutePath());
-            }
-            if (xmlURL != null) {
-                deployer.install(xmlURL, warURL);
-            } else {
-                deployer.install(path, warURL);
+        // There is a possible race condition here. If liveDeploy is true it
+        // means the deployer could start to deploy the app before we do it.
+        synchronized(getLock()) {
+            // Deploy this web application
+            try {
+                URL warURL =
+                    new URL("jar:file:" + localWar.getAbsolutePath() + "!/");
+                URL xmlURL = null;
+                if (localXml.exists()) {
+                    xmlURL = new URL("file:" + localXml.getAbsolutePath());
+                }
+                if (xmlURL != null) {
+                    deployer.install(xmlURL, warURL);
+                } else {
+                    deployer.install(path, warURL);
+                }
+            } catch (Throwable t) {
+                log("ManagerServlet.deploy[" + displayPath + "]", t);
+                writer.println(sm.getString("managerServlet.exception",
+                                            t.toString()));
+                localWar.delete();
+                localXml.delete();
+                return;
             }
-        } catch (Throwable t) {
-            log("ManagerServlet.deploy[" + displayPath + "]", t);
-            writer.println(sm.getString("managerServlet.exception",
-                                        t.toString()));
-            localWar.delete();
-            localXml.delete();
-            return;
         }
-
+        
         // Acknowledge successful completion of this deploy command
         writer.println(sm.getString("managerServlet.installed",
                                     displayPath));
@@ -1423,5 +1429,33 @@
 
     }
 
+    /**
+     * Obtain an object to lock on to prevent this servlet and the HostConfig
+     * thread both trying to deploy the same application.
+     * 
+     * @return An appropriate object to use for the lock
+     */
+    protected Object getLock() {
+        Object lock = null;
 
+        // Parent of context is host. Hosts have HostConfig lifecyclelisteners
+        Container parent = context.getParent();
+        if (parent instanceof Lifecycle) {
+            LifecycleListener[] listeners =
+                ((Lifecycle) parent).findLifecycleListeners();
+
+            for (int i=0; i<listeners.length; i++) {
+                if (listeners[i] instanceof HostConfig) {
+                    lock = listeners[i];
+                }
+            }
+        }
+        
+        // In case no HostConfig is found, can't sync on null
+        if (lock == null) {
+            lock = new Object();
+        }
+        
+        return lock;
+    }
 }

Modified: tomcat/container/branches/tc4.1.x/catalina/src/share/org/apache/catalina/startup/HostConfig.java
URL: http://svn.apache.org/viewvc/tomcat/container/branches/tc4.1.x/catalina/src/share/org/apache/catalina/startup/HostConfig.java?rev=413328&r1=413327&r2=413328&view=diff
==============================================================================
--- tomcat/container/branches/tc4.1.x/catalina/src/share/org/apache/catalina/startup/HostConfig.java (original)
+++ tomcat/container/branches/tc4.1.x/catalina/src/share/org/apache/catalina/startup/HostConfig.java Sat Jun 10 09:45:00 2006
@@ -24,6 +24,8 @@
 import java.net.URL;
 import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.Iterator;
+
 import javax.naming.NamingException;
 import javax.naming.directory.DirContext;
 import org.apache.naming.resources.ResourceAttributes;
@@ -328,7 +330,6 @@
             start();
         else if (event.getType().equals(Lifecycle.STOP_EVENT))
             stop();
-
     }
 
 
@@ -366,6 +367,8 @@
             return;
         String files[] = appBase.list();
 
+        checkDeployed();
+        
         deployDescriptors(appBase, files);
         deployWARs(appBase, files);
         deployDirectories(appBase, files);
@@ -374,6 +377,25 @@
 
 
     /**
+     * Check the list of files that have been auto-deployed to see if any of
+     * them have been removed, eg by the Manager app. If these files are left
+     * in the list of auto-deployed files then any subsequent attempt to
+     * redeploy them will fail.
+     */
+    protected void checkDeployed() {
+        ArrayList copy = (ArrayList) deployed.clone();
+        Iterator iter = copy.iterator();
+        while (iter.hasNext()) {
+            String filename = (String) iter.next();
+            File file = new File(appBase(), filename);
+            if (!(file.exists() || file.isDirectory())) {
+                deployed.remove(filename);
+            }
+        }
+    }
+    
+    
+    /**
      * Deploy XML context descriptors.
      */
     protected void deployDescriptors(File appBase, String[] files) {
@@ -436,7 +458,6 @@
             URL config = new URL("file", null, configFile.getCanonicalPath());
             stream = config.openStream();
             Digester digester = createDigester();
-            digester.setDebug(getDebug());
             digester.clear();
             digester.push(this);
             digester.parse(stream);
@@ -496,8 +517,11 @@
 
     /**
      * Deploy WAR files.
+     * 
+     * This method is synchronized to prevent a possible race condition
+     * with the manager servlet.
      */
-    protected void deployWARs(File appBase, String[] files) {
+    protected synchronized void deployWARs(File appBase, String[] files) {
 
         for (int i = 0; i < files.length; i++) {
 



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