| Summary: | Threading issues in RewriteHandler | ||
|---|---|---|---|
| Product: | [RT] Jetty | Reporter: | Gunnar Wagenknecht <gunnar> |
| Component: | server | Assignee: | Greg Wilkins <gregw> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | jesse.mcconnell, jetty-inbox |
| Version: | 7.1.4 | ||
| Target Milestone: | 7.1.x | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
Thanks Gunnar.. looking through things now cheers, jesse It seems that isHandle is actually just a synonym for Request#isHandled. RuleContainer only sets it together with the following code (RuleContainer.java, line 210). (request instanceof Request?(Request)request:HttpConnection.getCurrentConnection().getRequest()).setHandled(true); Maybe the code in RewriteHandle should check the request instead of '_rule.isHandled()' and the '_handled' logic can be removed in RuleContainer? yes, I am leaning that way as well I think that in a nutshell this could be a source of some intermittent failures to urls that ought to be rewritten or things that were not rewritten not being passed further up the handler chain... I think that is a nice catch :) ooops that was a bit of a howler! Fix in process. fix applied in r2211 for jetty-7. Not extensively tested yet. Also need to backport to jetty-6 fixed a while back, checking now to make sure it was resolved in jetty6 and if not will open an issue in jira for it |
I investigates RewriteHandler today for my use cases and while looking at its #handle implementation I think I found a threading issue. The implementation looks like this: 325: if (isStarted()) 326: { 327: String returned = _rules.matchAndApply(target, request, response); 328: target = (returned == null) ? target : returned; 329: 330: if (!_rules.isHandled()) 331: { 332: super.handle(target, baseRequest, request, response); 333: } 334: } Please correct me if I'm wrong. But it's #handle called from multiple to handle multiple parallel requests? '_rules' is a private member and shared between threads. The threading issue happends between '_rules.matchAndApply' (line 327) and '_rules.isHandled' (line 330). "_rules.isHandled" is implemented as a boolean field in RuleContainer. It maybe changed by a different thread and thus may not reflect a different state than the intended on (set by '_rules.matchAndApply'). Is this intended or an oversight? Should RuleContainer use a ThreadLocal?