Hi Christopher,

Am 22.07.2014 17:00, schrieb Christopher Schultz:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Simon,

On 7/22/14, 5:47 AM, Simon Kulessa wrote:
Hi Christopher,

see below

Am 17.07.2014 16:18, schrieb Christopher Schultz: Simon,

On 7/17/14, 3:52 AM, Simon Kulessa wrote:
Hi Christopher,

Am 16.07.2014 14:45, schrieb Christopher Schultz: Simon,

On 7/16/14, 4:02 AM, Simon Kulessa wrote:
Am 15.07.2014 16:12, schrieb Christopher Schultz:
Simon,

On 7/9/14, 4:51 AM, Simon Kulessa wrote:
I had a look at the documentation and the tomcat
source to get a better understanding of what the
'|org.apache.catalina.connector.RECYCLE_FACADE'
parameter actually does.|

I have seen that Tomcat objects like Cookies,
Request etc. are designed to be reusable.
Requests, yes. I haven't looked to see if Cookies would
be re-usable but it seems plausible.

What I currently do not understand is: In which
scenario and what context are they going to be
reused? I see there are Endpoints classes (like
NIOEndpoint) which are used to process the
different requests. This seems to be the most
likely entry point into the scenario.

Maybe somebody can provide some general outline
of how requests and the reusing of the object
actually works together? Is there some kind of
relation to the IP of an incoming request?
The client's IP is irrelevant: Tomcat uses a pool of
objects and re-fills each object with data from an
incoming request. These objects, as far as the web
application should be concerned, should have a valid
lifetime equal to the request itself. The servlet spec
requires these semantics, so this isn't some weird
Tomcat thing. Tomcat has chosen to pool these objects
for a small performance gain when it comes to memory
management and garbage collection.

If your application retains references to these objects
after they become invalid, they may contain invalid
data or valid data from another request after they
should have become invalid form the perspective of the
original request.

If you need data from a request, cookie, etc. then you
should copy it somewhere safe before the request ends.
We do not cache any request specific information. The
IP relation itself is irrelevant - it seems that the
reused object's contains more 'old' informations than
I previously assumed. The header itself and the
requestedSessionId seems to be changed, the sourceIP
and the cookie stay the same.
I checked, and Tomcat does not appear to cache Cookie objects
in any way. If the request object is cached and there is a
terrible bug, it might be possible to re-read an incorrect
session cookie from an old set of request headers.

At least the cookies have a recycle() method, which is
called if a request is recycled.
I don't see a recycle() method in the Cookie class. Where do you
see that?

I checked out the src for Tomcat the Tags 7.0.29 and 7.0.54 from
http://svn.apache.org/repos/asf/tomcat
In both version org.apache.tomcat.util.http.Cookies as well as
org.apache.tomcat.util.http.ServerCookie have a recycle() method
Aah, I see. The server does cache the "server cookie" structure, etc.,
but when parsing a request, the "server cookies" are copied into a
local (unshared) array with unshared javax.servlet.http.Cookie objects
in it. Those are the only objects available to the application.

See org.apache.catalina.connector.Request.parseCookies() for the code
that does that.

So, it's entirely possible that a request under the covers is
retaining cookie information from a previous request. If there is an
error during cookie parsing, I could see that a request would think
that the cookies had been (re)parsed when they hadn't been, but
recycling the request ought to set the "cookiesParsed" flag back to
false and so cookies ought to be re-parsed for that later request.
Also, in the event of an error, there should be some indication of the
error in the log file.

I don't even understand the "session id change" that you described
so it would be helpful to have a test case for that.

Basically it looks like this
0 request comes in via Servlet.doPost(...)
1 HttpSession session = request.getSession(); 2 String sId1 =
request.getSession().getId(); 3 session.invalidate(); 4 String
sId2 = request.getSession().getId();
5 response is written
Why are you doing 4 after 3? Are you trying to trigger the creation of
a new session cookie?
No that was not the intention here. It was just that the request passed as parameter to another method and nobody considered that the session should be transported
separately from this.

I change that now that the Session reference is only obtained once per request.
The session from 1 is saved in global map, so 3 is actually done
from another thread that has nothing to do with the request. The
other steps are done in same request and thread.
Problem is that 'sometimes' sId1 does not equal sId2.
I would never expect that they are the same: you are invalidating the
session and then asking for the session id: this will create a new
session, with a new id.

If you want to know if your session was invalidated (checking from
another thread), then you should call request.getSession(false) and
check for null. Since you are playing games with sessions accessed
from different places (which in general I think is mostly okay), then
you might also want to check to see if your session has been emptied
of all data (since there is no HttpSession.isValid method, but Tomcat
unbinds all attributes from the session when it is invalidated).

If step 4 is in fact calling request.getSession() instead of using a
local reference to the session obtained in step 1, then the session
returned should be a new one. If you are using a previously-obtained
reference from step 1, then you will have an invalid session object on
your hands.
Are you caching the AsyncContext yourself, or are you using an
AsyncListener to capture events and using the AsyncContext that
comes from the event?

We are caching the AsyncContext and we have a Listener as well to
react on the events. In the listener we mostly (with the
exception of onComplete, see below) use the cached reference, not
the one from the event.
I'm pretty sure that you are always going to want to use the context
from the event.

I'm not async expert (and I've never written any servlet 3.0
async-powered code) but you do have to be very careful with the
references you store and when you use them.

Can you even give us some pseudocode that shows what you are trying
to do? So far, all you've really said is "Tomcat is messing-up my
sessions". It took you 3 posts to mention that you were using
Servlet 3.0 async, which might be the most important piece of
information after the version you are using. Since your version is
old, you really should upgrade regardless of the outcome of this
conversation. In fact, there have been so many changes and tweaks
to the async-related code over the years (/literally/ years since
your release came out) that this problem may not even exist
anymore.
The code for starting the context looks like this:
// listener is an application scoped class that is used for all
AsyncContext's public void startAsync(HttpServletRequest
servletRequest, AsyncListener listener) {
AsyncContext asyncContext = servletRequest.startAsync(); if
(asyncContext == null) { throw new IllegalStateException(); }
HttpSession session = ((HttpServletRequest)
asyncContext.getRequest()).getSession(); if (session == null) {
asyncContext.complete(); throw new IllegalStateException(); }
asyncContext.addListener(listener); // store asyncContext in
global map, mapped to the current session }
This is likely when everything goes wrong: storing the async context
and session in a global map sounds like a recipe for disaster. Again,
I'm no expert, but my spidey sense is tingling.
You need the asyncContext to send something later.
This will happen while the httpRequest is still alive (somewhere in tomcat),
but after the Servlet has finished the processing.

In other example I see that some people use a running Thread to store
this reference in, but this should not be very different from what we are doing.

You have to maintain this reference somewhere. So by design there (hopefully) shouldn't
be any problem with 'caching' this object.

The async listener only covers the event's that occur, but the listener does not necessarily
have anything to do with the sending of an answer.

At this point of time the processing of the request which came
from a Servlet is done. The listener itself looks like this:
public void onTimeout(AsyncEvent event) throws IOException {
HttpSession httpSession = ((HttpServletRequest)
event.getSuppliedRequest()).getSession(false);
dispose(httpSession.getId()); }
Here, it looks like you are using the context from the event. This
usage should be okay.

public void onError(AsyncEvent event) throws IOException {
HttpSession httpSession = ((HttpServletRequest)
event.getSuppliedRequest()).getSession(false);
dispose(httpSession.getId()); }
private void dispose(String sessionId) { // retrieve related
session from global map AsyncContext ctx = ... if (ctx != null)
{ // Send some message that we close the session Message message
= ... sendMessageOnAsyncContext(message, ctx); }
httpSession.invalidate(); }
Instead of looking-up the context in the global map, why not just pass
the context into the dispose() method?

public void onComplete(AsyncEvent event) throws IOException {
// retrieve session based on the sessionId contained in the event
from the global map and take the AsyncContext from there
AsyncContext ctx = ...
I think you want to use the event's context, here.

if (event.getAsyncContext() == ctx) {  // <--- maybe equals would
be better here? but the object should have the same reference //
if same remove async context from global map } }
Under what conditions would you expect these async objects to be the
same/different? It looks like a sanity check, but you seem to be using
that check to trigger an important state-transition.
If a client loses the connection after a 1st asyncContext has been created
and sends another command to our server, then a 2nd asyncContext would be created and stored in the global Map.

The old one would be overwritten and the complete would be triggered for it.
In this case the context from the event will differ from the one stored in the global map.

For this reason as well we use the globally cached asyncContext in the other methods.

public void onStartAsync(AsyncEvent event) throws IOException {
// does nothing }
And then there is the sending of something which is used by the
listener and other places
// message is some internal used object to contain our message
public static void sendMessageOnAsyncContext(Message message,
AsyncContext context) throws IOException {
try { // send something on the context to inform the client that
we are closing it } finally { ctx.complete(); // remove async
context from global map } } }
Can you reproduce the problem in a test environment with 7.0.29?
Currently not
:(

So as of now your only options are:

1) Read every line of code in Tomcat to prove to yourself that there
was a bug in the old version and that it's been fixed in the new version
On this point, I mentioned this exception coming from Tomcat before:

... org.apache.coyote.AbstractProtocol$AbstractConnectionHandler process
SEVERE: null
java.lang.IllegalStateException: Calling [asyncPostProcess()] is not valid for a request with Async state [STARTED] at org.apache.coyote.AsyncStateMachine.asyncPostProcess(AsyncStateMachine.java:202) at org.apache.coyote.AbstractProcessor.asyncPostProcess(AbstractProcessor.java:116) at org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(AbstractProtocol.java:589) at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.run(NioEndpoint.java:1653)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
    at java.lang.Thread.run(Unknown Source)

Do you - now that you have looked at the recycling as well - see any connection between this exception
and the recycling?

It would interesting for me to actually reproduce this exception but without understanding the inner workings of Tomcat
I doubt I can do that.

2) Upgrade Tomcat in production and hope for the best
I am pretty sure, that setting the RECYCLE_FACADE flag to true (and therefore disabling the recycling) will prevent this issue from occurring. But this won't help me to track down the cause of this error.

??

In any case, I'd start using Tomcat 7.latest in your development and
test environments and upgrade production as soon as you deem it stable.

- -chris
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: GPGTools - http://gpgtools.org
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBCAAGBQJTznyPAAoJEBzwKT+lPKRY8BEQALTC6SBbVaPak3HO+VSkVmnL
vJYACtxk/0ZmaNmAeqQ5nYwIeFdIfy5yMUbNHMf8t+FSG4hhhk0cs4gQOljlZ1DY
TG2lyBD1xH80YiLVQb7v0KWi13maQ+59TS0AJusz955DYkGBxbzFI0UYNMkyYhIZ
OMNf+ALf6iVLwooXq6+NaM8QvAh6Zg3q4iqILgcR1RR7BHSBMMb4uXzR1NYPHBKk
PAmWsAY1HtegeKwANRnVaGgtZKaKO+95y41RLMbv9cQ/3fj5pRRzgM+1esspR880
zTkEu+zj4EKbY7dUpWYr0g8A1o+ECKuRa7jtdtdoqgMYvjkGSbGFkQT172FHNqzI
H3h3JI7UxMLOMA8Ex1ysWvnDinewV0pWf6SQgSeotoYk6Z7usT7BCxRgq+bXXCTw
xLNkJ6tM2ft/kDpjjvbxv3fSrOd8Sr6eQ6TFgymfpTmHNhWY4DzhRPzdaiwBkf/3
xcw8Q2W2Rd9566v9UdxxHmYn0enFkr3HgxJeFpl2UnLWplVo3me9XZ0rRIO/bdS2
pOmAzjL+cgAUcubXfKL2vAyMo0yomquodZ/eG4Lm0MojymfQMrrZM1zj1QzKvPhi
qUXCQp07Ubwucj+ML226J89iiElkXdF3OfqJDvrfTAxcRxJiuPJKgtpxfwOLy5/V
nebJTCQ7reqv5ztrGsSH
=t7vf
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org


Regards,
Simon

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org

Reply via email to