Bug 56653 - Concurrency issue with Mapper$ContextList when stopping Contexts
Summary: Concurrency issue with Mapper$ContextList when stopping Contexts
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 6
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 6.0.41
Hardware: PC All
: P2 normal (vote)
Target Milestone: default
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-06-21 02:19 UTC by Konstantin Kolinko
Modified: 2017-01-01 11:15 UTC (History)
0 users



Attachments
2014-06-21_tc8_56653_v1.patch (4.84 KB, patch)
2014-06-21 03:10 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 2014-06-21 02:19:53 UTC
I noticed this issue while reviewing the code of Mapper.removeContextVersion() of the current trunk (@1604217). The same code exists in Tomcat 7 and 6.

In Mapper.removeContextVersion() (Mapper.removeContext() in Tomcat 6) it does the following:
[[[
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;       
    }                                                
}                                                    
]]]

The problem is there is a delay between when the list of contexts is updated (contextList.contexts) and the contextList.nesting field is updated. The "nesting" field is used when mapping contexts.

For example,
1. If there are the following contexts:
ROOT
foo
foo#bar

2. Context foo#bar is being stopped.

3. A request for "foo" comes in, e.g. http://localhost/foo/index.html
Expected behaviour: Map the context to foo application.
Actual behaviour:
It may be that the request will be erroneously mapped to the ROOT webapp instead of "foo".

I have a test case.
Comment 1 Konstantin Kolinko 2014-06-21 02:26:35 UTC
I added the test case in r1604309.
It may pass, but I also observed the following failures:

org.junit.ComparisonFailure: expected:<[/foo/bar/bla]> but was:<[]>
	at org.apache.catalina.mapper.TestMapper.testContextListConcurrencyBug56653(TestMapper.java:263)
Comment 2 Konstantin Kolinko 2014-06-21 03:10:03 UTC
Created attachment 31736 [details]
2014-06-21_tc8_56653_v1.patch

This patch fixes the issue, BUT breaks aliases support.
Thanks to TestMapper tests for catching that.

The idea to turn ContextList into an immutable object with final fields does not work, because all aliases share the same ContextList object.
Comment 3 Konstantin Kolinko 2014-06-21 06:31:07 UTC
Fixed in Tomcat 8 by r1604319 and will be in 8.0.10 onwards.
Comment 4 Konstantin Kolinko 2014-06-23 21:19:11 UTC
(In reply to Konstantin Kolinko from comment #3)
> Fixed in Tomcat 8 by r1604319 and will be in 8.0.10 onwards.

A part of that solution was reverted in r1604934 and re-implemented in a different way in r1604940.

Instead of introducing an additional level of indirection, I run a loop to update all the aliases.
Comment 5 Konstantin Kolinko 2014-07-09 02:38:16 UTC
Fixed in Tomcat 7 with r1608983 and r1608993 and  will be in 7.0.55 onwards.
Comment 6 Mark Thomas 2017-01-01 11:15:50 UTC
Marking as fixed since this was fixed in 7.0.x.

Note it was not back-ported to 6.0.x since 6.0.x reached end of life before anyone expressed an interest in back-porting the fix.