-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Mark,
On 12/10/19 03:26, Mark Thomas wrote:
> On 09/12/2019 19:54, Jerry Malcolm wrote:
>
> <snip/>
>
>> Thanks to all of the responders on this problem. From the
>> beginning of this problem, I was convinced that I was doing
>> something bad wrong to cause this. I think now I have finally
>> identified the 'bad thing'. Periodically the model code that runs
>> in the request determines that I need to spin off another thread
>> to do some background work. That thread needs a few parameters
>> from the request object. So I pass the request object into the
>> thread object. I realize now that doing that is a horrible thing
>> to do. I was not aware until yesterday that request objects are
>> pooled and recycled. So I figured holding onto the request
>> object a little longer before it is discarded for that thread to
>> use was not going to be a problem. Obviously, that was
>> incorrect.
>
> Yep, that'll do it.
>
>> From what I can deduce, the main request thread finishes and
>> returns. The request object recycle code runs and the object is
>> returned to the pool. However, then the async thread later gets
>> a parm and must apparently still be able to use the request
>> object even though it's back in the pool. The
>> 'parametersProcessed' gets set to true. Then the next request
>> comes in, and that request object is assigned with
>> parametersProcessed=true.
>
> I think this analysis is correct.
>
>> I'm assuming the fix is to get everything I need out of the
>> request object and pass those parm values to the thread before
>> returning instead of passing the request object itself to the
>> thread.
>
> Correct.
>
>> Does all of this sound plausible as the source for this. The
>> good news is that I'm still teachable... :-)
>
> Very plausible.
>
> Kudos for figuring this out so quickly - and for the very clear
> write up.
Would using org.apache.catalina.connector.RECYCLE_FACADES=true have
made this problem go away? Or would the behavior have been the same,
just less dangerous?
I'm wondering if Tomcat could or should have another safety feature to
help catch this sort of thing in development. In all my development
environments, I have the JDBC connection pool size set to a fixed
maximum of 1 connection. This means that any potential deadlocks in
the application due to sloppy connection-management will cause pretty
early because we'll get pool-fetch timeouts, missing-return-connection
errors, etc.
Request object reuse has a measurable positive effect on performance
in production, but in development probably doesn't matter quite so
much. In the same way that WebappClassLoader becomes inert when the
application has been stopped, perhaps we could "shut-down" request /
response / session objects that have been loaned to a
request-processor thread.
Something like this:
Request realRequest = ... // fetch pooled request object
HttpServletRequest req;
RecyclableWrapper<HttpServletRequest> wrapper;
if(developmentSafetyMode) {
wrapper = new RecyclableWrapper<HttpServletRequest>(realRequest);
req =
(HttpServletRequest)Proxy.newProxyInstance(MigrateConfiguration.class.ge
tClassLoader(),
new Class[] { HttpServletRequest.class }, wrapper);
} else {
req = realRequest;
}
// dispatch request
if(null != wrapper) {
wrapper.recycle();
}
And then the invocation handler class:
static class RecyclableWrapper<T>
implements InvocationHandler
{
private T target;
private boolean recycled;
public RecyclableWrapper(T target) {
this.target = target;
}
public void recycle() {
recycled = true;
}
@Override
public Object invoke(Object proxy, Method method, Object[] args)
throws Throwable {
if(recycled)
throw new InvocationTargetException((Throwable)null,
"Object has been recycled.");
return method.invoke(target, args);
}
}
This would catch these kinds of errors very early, and we could use a
very specific error message that is easy to find using e.g. Google. OR
just put a URL to a wiki page explaining the problem directly in the
error message.
I suppose this could even be done as a Valve (or Filter) so it's
completely optional and trivially configurable. If a Valve, we'd have
to wrap the Request instead of HttpServletRequest.
I've only shown how to wrap the request above, but of course the
response, session, and maybe even the streams should be wrapped, too,
to be completely safe.
Is anyone interested in this kind of thing being packaged with Tomcat?
- -chris
-----BEGIN PGP SIGNATURE-----
Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/
iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAl3vqzkACgkQHPApP6U8
pFg2qg/+PhgRgGxELAoqv/ZHPBjpv5IutL1X+uLa52Nl3NyuIPSWTFV7SB+ORE0l
5x8DVjW7MR7OUuu4Ptg2RPVmrufWDSANfujtJ9RS/ugzAii33O+3MIY2zHAZgfMk
AKmcIQ3eGK4Ep1SByPTvD7nK3nmIlSzn7Z6d4bBeSawOFYcb1i1zs026gxfgTnnq
x98+bl52YCDi9wCZk8GrglQ1eRy036K7WvmiEAVNs9SZNIhoho1tzvJOYmZ5WT3D
OQ0xOwpecf3S84Mi5BdNppNMOlBCGSgYhN99/LtMHA+vFV61SYFQhTxXt+151o4s
9pgFlZfdUFGqoA0EBaKX8/42NKOmifeSjirsoeOhSY91x1ZMix9IX0s8dlzvKEQZ
XYytge9BUsIhcKAT9Cw4+YGXeFD3HPIDpjfQrRPlWEjPmGv9eOtuCRySdrGxEea3
njr5LWO54ISD0vqeXVY3enWkHeoDUJORBoERHBGF33duUGy/nQzgPrOTaMwOkUWK
evU7pGe/LbBUaSBqXFXWdA8aLHLqzFJ8aM+8mPnvZjBH/jQh6QOtthTcZZlOw8ZH
ndzeDGLeBXK+cfX+87AymE5VmV2RShsafs8omjX+LLz8DMry99wEkVAIBYPAMV6I
mZaH5ad4f6wuex//vcLdDIAuFDchfymtt95mJIAP+y/uBNxek2A=
=XZM9
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org
