-----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? >> 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. >> 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. >> 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 2) Upgrade Tomcat in production and hope for the best ?? 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
