Community
Participate
Working Groups
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?
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
