Summary: | Loss of data and concurrency issue with Catalina session persistent storage | ||
---|---|---|---|
Product: | Tomcat 5 | Reporter: | Wade Chandler <hwadechandler-apache> |
Component: | Catalina | Assignee: | Tomcat Developers Mailing List <dev> |
Status: | RESOLVED FIXED | ||
Severity: | normal | ||
Priority: | P2 | ||
Version: | 5.5.27 | ||
Target Milestone: | --- | ||
Hardware: | Other | ||
OS: | other | ||
Attachments: |
Reproducer, part 1: web application
Reproducer, part 2: client |
Description
Wade Chandler
2007-09-10 10:54:45 UTC
I thought about marking this as a P1 as it could cause some very hard to track down data loss in a web application. What is the difference between P1 and P2 for the Tomcat project? Using a plain Object to synchronize on rather than an Integer(0) would be very slightly more efficient. Any reason not to use the Session object itself as the monitor? No real reason I can find. We might want to use an instance variable as the lock just in case there are ever any other synchronized methods or sections added to the session where the entire session is locked. That would help if those synchronized session methods would need to be used by other threads and could be accessed even during passivation. I only find one place where the session itself is locked and that is in the expires method where a synchronized (this) is used. So, using the session itself would be OK for now, and it may logically work out to be the case that no other action should ever be able to actually be made or ever wanted on the session while it is being cached/passivated. I mentioned it could be used as the lock in the my description some where. So, at least for now, or always if deemed to always be wanted, I don't see any reason why it itself can not be the lock. I have applied a patch to trunk based on the discussion here and proposed it for 6.0.x. The patch has been applied to 6.0.x and will be in 6.0.17 onwards. Thanks for the suggested patch. Created attachment 23592 [details]
Reproducer, part 1: web application
Web application for TC 5.5, 6.0 to reproduce the issue
Created attachment 23593 [details] Reproducer, part 2: client HTTPClient. commons-httpclient-3.1.jar and its dependencies are not included. To run: 1. Deploy bug43343.war on a web server 2. Unpack this bug43343_client.zip and put the following libraries into lib subfolder: commons-httpclient-3.1.jar commons-codec-1.3.jar commons-logging-1.1.1.jar 3. Run client.bat if you are on Windows The client creates several (100) instances of HttpClient that connect to http://localhost:8080/bug43343/index.jsp?number=nn&loop=ll Each HttpClient remembers its cookies, and thus connects to a separate session. The number parameter in URL is unique for each HttpClient instance, and loop starts with 0 and increments by 1 for each request from the same client. The context.xml file of the web application configures PersistentManager with some low values, to that this issue is more visible. The requests are being sent serially, with an interval of at least 4 seconds between request of the same client. The client prints out the response of web application, and terminates if status code 500 is received. Expected result: Successful run of the client. That is, 150 loops, which needs about 10 minutes to complete. I am reopening this issue, because: 1. The fix was not applied to TC 5.5, and the issue can be reproduced with TC 5.5.27. 2. The fix [1], as applied to 6.0 is not complete, though one may argue. I am reducing severity to "normal". The issue does not apply to the default configuration, where StandardManager is used. [1] http://svn.apache.org/viewvc?rev=656751&view=rev On the issue: When PersistentManagerBase swaps out a session it performs two actions (see its implementation of swapOut()): 1. writing session data out 2. calling session.recycle() which invalidates the session When session that is being used by an active request is being swapped out at the same time (and what is the need for doing that? - you will not be able to free the memory, because there is a reference), two races can occur: Case A: request writes data to session at the same time as it is being written out and before the recycle() call. Result: the update is lost, next request will see the old values. This is the case that is fixed by PersistentManagerBase patch in rev=656751 [1]. Case B: request accesses session after session.recycle(), e.g. after swapOut() completes. Result: java.lang.IllegalStateException: setAttribute: Session already invalidated, though the session was not invalidated, but just swapped out. --------------------------------------------- Sample output from the client when Case A is observed (with TC 5.5.27): Method failed: HTTP/1.1 500 Internal Server Error Error (number=-1509748992, loop=140): java.lang.IllegalStateException: Attribute loop is not equal to the expected val ue. Expected: 139 Actual: 138 --------------------------------------------- Case B can be reproduced more easily if you replace s/ if (false) { / if (true) { / on line 57 in file index.jsp of the web application. Sample output from the client when Case B is observed (current tc6.0.x, with the mentioned change of line 57): Method failed: HTTP/1.1 500 Internal Server Error Error (number=-1309785265, loop=0): java.lang.IllegalStateException: setAttribute: Session already invalidated I have also seen the following NullPointerException when running against TC 5.5.27, and I think that it is the Case B: The server encountered an internal error () that prevented it from fulfilling this request. exception: org.apache.jasper.JasperException org.apache.jasper.servlet.JspServletWrapper.handleJspException(JspServle tWrapper.java:460) org.apache.jasper.servlet.JspServletWrapper.service(JspServletWrapper.ja va:373) org.apache.jasper.servlet.JspServlet.serviceJspFile(JspServlet.java:329) org.apache.jasper.servlet.JspServlet.service(JspServlet.java:265) javax.servlet.http.HttpServlet.service(HttpServlet.java:729) root cause: java.lang.NullPointerException org.apache.catalina.session.StandardSession.setAttribute(StandardSession .java:1309) org.apache.catalina.session.StandardSession.setAttribute(StandardSession .java:1248) org.apache.catalina.session.StandardSessionFacade.setAttribute(StandardS essionFacade.java:130) org.apache.jsp.index_jsp._jspService(index_jsp.java:88) org.apache.jasper.runtime.HttpJspBase.service(HttpJspBase.java:98) javax.servlet.http.HttpServlet.service(HttpServlet.java:729) org.apache.jasper.servlet.JspServletWrapper.service(JspServletWrapper.ja va:331) org.apache.jasper.servlet.JspServlet.serviceJspFile(JspServlet.java:329) org.apache.jasper.servlet.JspServlet.service(JspServlet.java:265) javax.servlet.http.HttpServlet.service(HttpServlet.java:729) --------------------------------------------- Case B can be solved by avoiding swapping out sessions that are owned by a request. Synchronizing on a session is not sufficient. A workaround is to avoid enabling IdleSwap in the configuration (it is disabled by default). Case B fixed in trunk and proposed for 6.0.x Complete fix proposed for 5.5.x. This has been fixed for 6.0.x and will be included in 6.0.21 onwards. Transferring to TC5 so it can be used to track the fix there. This has been fixed in 5.5.x and will be included in 5.5.28 onwards. |