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 2013/05/21 22:01:03 UTC
svn commit: r1484923 -
/tomcat/tc6.0.x/trunk/java/org/apache/catalina/startup/ContextConfig.java
Author: markt
Date: Tue May 21 20:01:02 2013
New Revision: 1484923
URL: http://svn.apache.org/r1484923
Log:
Make deletion of the copied WARs used for anti-resource locking more robust if the context fails to start (there were some circumstances where the original WAR could get deleted).
Modified:
tomcat/tc6.0.x/trunk/java/org/apache/catalina/startup/ContextConfig.java
Modified: tomcat/tc6.0.x/trunk/java/org/apache/catalina/startup/ContextConfig.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/startup/ContextConfig.java?rev=1484923&r1=1484922&r2=1484923&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/startup/ContextConfig.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/startup/ContextConfig.java Tue May 21 20:01:02 2013
@@ -127,6 +127,12 @@ public class ContextConfig
/**
+ * Anti-locking docBase
+ */
+ private String antiLockingDocBase = null;
+
+
+ /**
* The string resources for this package.
*/
protected static final StringManager sm =
@@ -264,16 +270,9 @@ public class ContextConfig
} else if (event.getType().equals(StandardContext.AFTER_START_EVENT)) {
// Restore docBase for management tools
if (originalDocBase != null) {
- String docBase = context.getDocBase();
context.setDocBase(originalDocBase);
- originalDocBase = docBase;
}
} else if (event.getType().equals(Lifecycle.STOP_EVENT)) {
- if (originalDocBase != null) {
- String docBase = context.getDocBase();
- context.setDocBase(originalDocBase);
- originalDocBase = docBase;
- }
stop();
} else if (event.getType().equals(Lifecycle.INIT_EVENT)) {
init();
@@ -931,8 +930,7 @@ public class ContextConfig
}
- protected void antiLocking()
- throws IOException {
+ protected void antiLocking() throws IOException {
if ((context instanceof StandardContext)
&& ((StandardContext) context).getAntiResourceLocking()) {
@@ -942,11 +940,8 @@ public class ContextConfig
String docBase = context.getDocBase();
if (docBase == null)
return;
- if (originalDocBase == null) {
- originalDocBase = docBase;
- } else {
- docBase = originalDocBase;
- }
+ originalDocBase = docBase;
+
File docBaseFile = new File(docBase);
if (!docBaseFile.isAbsolute()) {
File file = new File(appBase);
@@ -983,14 +978,13 @@ public class ContextConfig
log.debug("Anti locking context[" + context.getPath()
+ "] setting docBase to " + file);
+ antiLockingDocBase = file.getAbsolutePath();
// Cleanup just in case an old deployment is lying around
ExpandWar.delete(file);
if (ExpandWar.copy(docBaseFile, file)) {
- context.setDocBase(file.getAbsolutePath());
+ context.setDocBase(antiLockingDocBase);
}
-
}
-
}
@@ -1264,8 +1258,8 @@ public class ContextConfig
Host host = (Host) context.getParent();
String appBase = host.getAppBase();
String docBase = context.getDocBase();
- if ((docBase != null) && (originalDocBase != null)) {
- File docBaseFile = new File(docBase);
+ if (antiLockingDocBase != null) {
+ File docBaseFile = new File(antiLockingDocBase);
if (!docBaseFile.isAbsolute()) {
docBaseFile = new File(appBase, docBase);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1484923 - /tomcat/tc6.0.x/trunk/java/org/apache/catalina/startup/ContextConfig.java
Posted by Mark Thomas <ma...@apache.org>.
On 21/05/2013 23:10, Konstantin Kolinko wrote:
> 2013/5/22 <ma...@apache.org>:
>> Author: markt
>> Date: Tue May 21 20:01:02 2013
>> New Revision: 1484923
>>
>> URL: http://svn.apache.org/r1484923
>> Log:
>> Make deletion of the copied WARs used for anti-resource locking more robust if the context fails to start (there were some circumstances where the original WAR could get deleted).
>> @@ -942,11 +940,8 @@ public class ContextConfig
>> String docBase = context.getDocBase();
>> if (docBase == null)
>> return;
>> - if (originalDocBase == null) {
>> - originalDocBase = docBase;
>> - } else {
>> - docBase = originalDocBase;
>> - }
>> + originalDocBase = docBase;
>> +
>
> 2. Why if(originalDocBase == null) check was removed?
It was part of the rotating the original and modified values. Now we
just keep copies of both it the check is irrelevant.
> 1. This change in 6.0 needs to go through voting
Sorry - I was on automatic pilot when doing my back-ports.
Clearly there is a +1 from me. If two other folks +1 it fairly soon I'll
just leave it. If not, I'll revert it and add it to the status file.
> 2. There is a bug in the above line,
> "docBase" should not be there.
> (Though it never executes, as antiLockingDocBase was created as
> file.getAbsolutePath(),)
> It will allow to simplify this block a bit.
I'll clean that up in trunk and 7.0.x and propose it for 6.0.x.
> 3. Wouldn't it be more simple to have the new field as a File instead of String?
> While "docBase" is a String, per API, this field represents a proper file.
Yes. That allows a few more lines to be removed.
Thanks for the review.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1484923 - /tomcat/tc6.0.x/trunk/java/org/apache/catalina/startup/ContextConfig.java
Posted by Konstantin Kolinko <kn...@gmail.com>.
2013/5/22 <ma...@apache.org>:
> Author: markt
> Date: Tue May 21 20:01:02 2013
> New Revision: 1484923
>
> URL: http://svn.apache.org/r1484923
> Log:
> Make deletion of the copied WARs used for anti-resource locking more robust if the context fails to start (there were some circumstances where the original WAR could get deleted).
>
> Modified:
> tomcat/tc6.0.x/trunk/java/org/apache/catalina/startup/ContextConfig.java
>
> Modified: tomcat/tc6.0.x/trunk/java/org/apache/catalina/startup/ContextConfig.java
> URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/startup/ContextConfig.java?rev=1484923&r1=1484922&r2=1484923&view=diff
> ==============================================================================
> --- tomcat/tc6.0.x/trunk/java/org/apache/catalina/startup/ContextConfig.java (original)
> +++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/startup/ContextConfig.java Tue May 21 20:01:02 2013
> @@ -127,6 +127,12 @@ public class ContextConfig
>
>
> /**
> + * Anti-locking docBase
> + */
> + private String antiLockingDocBase = null;
> +
> +
> + /**
> * The string resources for this package.
> */
> protected static final StringManager sm =
> @@ -264,16 +270,9 @@ public class ContextConfig
> } else if (event.getType().equals(StandardContext.AFTER_START_EVENT)) {
> // Restore docBase for management tools
> if (originalDocBase != null) {
> - String docBase = context.getDocBase();
> context.setDocBase(originalDocBase);
> - originalDocBase = docBase;
> }
> } else if (event.getType().equals(Lifecycle.STOP_EVENT)) {
> - if (originalDocBase != null) {
> - String docBase = context.getDocBase();
> - context.setDocBase(originalDocBase);
> - originalDocBase = docBase;
> - }
> stop();
> } else if (event.getType().equals(Lifecycle.INIT_EVENT)) {
> init();
> @@ -931,8 +930,7 @@ public class ContextConfig
> }
>
>
> - protected void antiLocking()
> - throws IOException {
> + protected void antiLocking() throws IOException {
>
> if ((context instanceof StandardContext)
> && ((StandardContext) context).getAntiResourceLocking()) {
> @@ -942,11 +940,8 @@ public class ContextConfig
> String docBase = context.getDocBase();
> if (docBase == null)
> return;
> - if (originalDocBase == null) {
> - originalDocBase = docBase;
> - } else {
> - docBase = originalDocBase;
> - }
> + originalDocBase = docBase;
> +
2. Why if(originalDocBase == null) check was removed?
> File docBaseFile = new File(docBase);
> if (!docBaseFile.isAbsolute()) {
> File file = new File(appBase);
> @@ -983,14 +978,13 @@ public class ContextConfig
> log.debug("Anti locking context[" + context.getPath()
> + "] setting docBase to " + file);
>
> + antiLockingDocBase = file.getAbsolutePath();
> // Cleanup just in case an old deployment is lying around
> ExpandWar.delete(file);
> if (ExpandWar.copy(docBaseFile, file)) {
> - context.setDocBase(file.getAbsolutePath());
> + context.setDocBase(antiLockingDocBase);
> }
> -
> }
> -
> }
>
>
> @@ -1264,8 +1258,8 @@ public class ContextConfig
> Host host = (Host) context.getParent();
> String appBase = host.getAppBase();
> String docBase = context.getDocBase();
> - if ((docBase != null) && (originalDocBase != null)) {
> - File docBaseFile = new File(docBase);
> + if (antiLockingDocBase != null) {
> + File docBaseFile = new File(antiLockingDocBase);
> if (!docBaseFile.isAbsolute()) {
> docBaseFile = new File(appBase, docBase);
1. This change in 6.0 needs to go through voting
2. There is a bug in the above line,
"docBase" should not be there.
(Though it never executes, as antiLockingDocBase was created as
file.getAbsolutePath(),)
It will allow to simplify this block a bit.
3. Wouldn't it be more simple to have the new field as a File instead of String?
While "docBase" is a String, per API, this field represents a proper file.
The rest of the change is OK.
(It changes the way how "originalDocBase" field is handled, but I
would say that it is OK. The new behaviour makes more sense. Regarding
lifecycleEvent() method, now it goes as:
a) BEFORE_START_EVENT -> beforeStart()
-> call antiLocking() -> save docBase into originalDocBase field
b) START_EVENT -> (...)
c) AFTER_START_EVENT -> restore docBase
Thus there was OK to remove that "restoring" code from STOP_EVENT block.
)
> }
>
>
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org