Bug 54007 - Improve handling of failed web application deployments
Improve handling of failed web application deployments
Status: NEW
Product: Tomcat 6
Classification: Unclassified
Component: Catalina
PC Windows XP
: P2 normal (vote)
: default
Assigned To: Tomcat Developers Mailing List
Depends on:
  Show dependency tree
Reported: 2012-10-15 09:03 UTC by Konstantin Kolinko
Modified: 2012-11-12 01:27 UTC (History)
0 users

2011-11-14_tc6_54007.patch (19.41 KB, patch)
2012-10-15 09:15 UTC, Konstantin Kolinko
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Konstantin Kolinko 2012-10-15 09:03:18 UTC
1) If Tomcat 6 runs with autodeployment being enabled, and a web application deployment fails, Tomcat will repeat attempts to autodeploy the application every 10 seconds. So every 10 seconds an error message is printed into the error log.

To reproduce: place a broken context xml file (e.g. with a typo) into conf/Catalina/localhost.

2) The input stream for the broken context xml file is not properly closed. When running on Windows the file cannot be deleted without stopping Tomcat.

3) If a broken web application is deployed via Manager web application GUI, it fails to deploy and it is not displayed in the list of the applications. 

As a consequence
- It cannot be undeployed via GUI, as it is absent from the list. If one crafts the command URL manually, it fails with
"FAIL - No context exists for path /test"
- It cannot be replaced with a new correct version of the application, because if you try to upload a new war, it fails with
"FAIL - War file "test.war" already exists on server"

Note, that in Tomcat 7 the issue has already been fixed in 7.0.23
Comment 1 Konstantin Kolinko 2012-10-15 09:15:07 UTC
Created attachment 29477 [details]

Patch for Tomcat 6

The change in ContainerBase.addChild() is needed to display failed applications in the Manager webapp.
Comment 2 Konstantin Kolinko 2012-10-15 11:21:36 UTC
Concerns about the patch
1) Messy. The new methods have to be documented better and to be made protected instead of private.

2) Several concerns about getContextPathFromFileName() method:

- It needs better API.

In the patch it combines context name calculation, canonicalization check and update of invalidWars map. Originally the check was supposed to be performed for war files only.

- Typo in getContextPathFromFileName() JavaDoc:
 a Cyrillic char instead of trailing '.'

- A bug:
The "appbase" argument is not used because of a typo.
The method calls validateContextPath(appBase,..) using this.appBase instead of "appbase" argument.

- The validation check that uses canonicalization is expensive. It could be improved by reordering the checks. Reordering cannot be done if the check is encapsulated in this method. See

3) HostConfig.invalidWars is a HashSet.
I think it has to be made final and access to it has to be made synchonized.

4) As noted by kfujino in STATUS.txt of Tomcat 6 where this patch was proposed:

 Question of if host.addChild(context) threw IllegalStateException.
 E.g. case of deployDirectory.
 If META-INF/context.xml exist in Directory, context.xml is copied to configBase.
 If host.addChild(context) threw IllegalStateException, only a directory is 
 registered into redeployResources. Context.xml does not register into redeployResources.
 If manager app execute undeploy(or delete directory), directory is deleted 
 but context.xml isn't deleted.
 Should (conf/Catalina/localhost/)context.xml be registered into redeployResources? 
 Or need to delete context.xml manually?

I think context.xml should be registered into redeployResources.

A better patch is needed.

5) Regarding the change to ContainerBase.addChild()

It seems too wide (concerns all containers), but I do not see a better solution here.

(In reply to comment #0)
> 2) The input stream for the broken context xml file is not properly closed.
> When running on Windows the file cannot be deleted without stopping Tomcat.

I do not have a solution for the locked context xml file issue. Tomcat 7 is affected as well. The issue seems to occur inside XmlReader created by Digester. We could patch out copy of Digester, but I do not see an API there to explicitly close the file.

The workaround is to force full GC (e.g. to press the "Find Leaks" button in the Manager). After that the file is unlocked.
Comment 3 Mark Thomas 2012-11-12 01:27:39 UTC
Note the leak has been identified and fixed in trunk and 7.0.x. See r1408163 and r1408166.