Bug 56190 - Connection keep-alive not working with asynchronous servlet
Summary: Connection keep-alive not working with asynchronous servlet
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 8
Classification: Unclassified
Component: Specification APIs (show other bugs)
Version: 8.0.3
Hardware: PC Linux
: P2 normal (vote)
Target Milestone: ----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-02-26 13:52 UTC by Francois-Xavier Bonnet
Modified: 2014-03-14 16:51 UTC (History)
0 users



Attachments
AsyncHelloServlet.java (977 bytes, text/plain)
2014-02-26 13:52 UTC, Francois-Xavier Bonnet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Francois-Xavier Bonnet 2014-02-26 13:52:07 UTC
Created attachment 31346 [details]
AsyncHelloServlet.java

I am trying to use asynchronous servlets. It looks like whenever you call request.startAsync() keep-alive is disabled and the response is sent in chunked transfer encoding.

I attached the code of a simple servlet. I get these headers in the response:
HTTP/1.1 200 OK
Server: Apache-Coyote/1.1
Transfer-Encoding: chunked
Date: Wed, 26 Feb 2014 13:25:29 GMT

With a regular blocking servlet I get:
HTTP/1.1 200 OK
Server: Apache-Coyote/1.1
Content-Length: 11
Date: Wed, 26 Feb 2014 13:49:17 GMT
Comment 1 Francois-Xavier Bonnet 2014-02-26 13:53:01 UTC
I have the same issue with Tomcat 7.0.52
Comment 2 Konstantin Kolinko 2014-02-27 11:33:01 UTC
> I get these headers in the response

With those headers you have keep-alive enabled (as that is default if nothing else is specified in an HTTP/1.1 response, per HTTP/1.1 specification).

So, what is your problem?

>  response is sent in chunked transfer encoding

"Chunked encoding" just means that response length was not know at the moment when response had to be sent to client. E.g. if output stream was flushed before closing the connection. Both are valid responses.
Comment 3 Mark Thomas 2014-02-27 12:10:28 UTC
This is as per the Servlet specification. The response has to committed when the async context is completed. At this point the response body may not be fully written so chunked encoding has to be used.
Comment 4 Francois-Xavier Bonnet 2014-02-28 11:54:45 UTC
Thanks for your answers. I had a closer look at the specifications and there is no problem about the keep-alive header.

About the "Transfer-Encoding: chunked" header, in my test I am just writing "Hello world" in the response (should be small enough for the buffer), there is no flush so when I call AsyncContext.complete() method  the content size is perfectly known and I would expect the server to set the content-length header and not to use chunked content encoding.
According to servlet 3.0 specification "The content length is automatically set if the entire response fits inside the response buffer."
http://docs.oracle.com/javaee/6/api/javax/servlet/http/HttpServlet.html#doGet(javax.servlet.http.HttpServletRequest, javax.servlet.http.HttpServletResponse)

Just to explain how I found this issue: I was doing some tests with Apache Benchmark. I was surprised to see that the performance of Tomcat were not good compared to other servlet containers, then I noticed that keep-alive was not working with tomcat while it was with other servlet containers like Jetty. It looks like ab disables keep-alive when the response is chunked.
Comment 5 Mark Thomas 2014-02-28 11:58:09 UTC
Please re-read comment#3
Comment 6 Francois-Xavier Bonnet 2014-02-28 12:07:58 UTC
I did.
I know the response is committed when you call AsyncContext.complete() but at this point the response body is fully written.
The specification is clear about that: "Completes the asynchronous operation that was started on the request that was used to initialze this AsyncContext, closing the response that was used to initialize this AsyncContext."

http://docs.oracle.com/javaee/6/api/javax/servlet/AsyncContext.html#complete()
Comment 7 Konstantin Kolinko 2014-03-03 11:34:45 UTC
1. I agree that "closing the response that was used to initialize this AsyncContext." seems to be missing from AsyncContextImpl.complete()

[[[
 request.getCoyoteRequest().action(ActionCode.COMMIT, null);
 request.getCoyoteRequest().action(ActionCode.ASYNC_COMPLETE, null);
 clearServletRequestResponse();
]]]


2. It is unclear whether this "commit" and "closing the response" have to occur before firing onComplete() event or after it.

It it has to be committed before firing and closed after firing, you would get the chunked response.

To get a non-chunked response, one has to close the response without an explicit commit. Either (close before firing) or (non commit before firing + close after firing).

I am reading that onComplete() is designed to allow cleanup of some resources, so I think that response can be closed before firing. (Though it will break applications that expect otherwise. Are there are any?)


3. There is a workaround for your example:
Close the response explicitly,
[[[
  response.getWriter().close()
]]]

You should get a non-chunked response this way.
Comment 8 Mark Thomas 2014-03-05 21:38:10 UTC
I've been back and re-read both the Servlet 3.0 and Servlet 3.1 specifications. I don't know where I got the idea from that the response always had to be writable after the call to complete() but that is clearly wrong.

There is the requirement that when called before the container-initiated dispatch that called startAsync() has returned to the container then the complete() call doesn't take effect until after that thread returns but when it does take effect it still needs to close the response.

I'll take a look at getting this fixed.

This might break some applications (it might even break some of our test cases) but the spec is clear on the required behavior so I think any such breakage is acceptable.
Comment 9 Mark Thomas 2014-03-06 09:31:06 UTC
I have the first pass at a fix for this ready. The required changes are minimal but it breaks a lot of unit tests. The unit tests depend on being able to write to the response after the async complete. I need to implement an alternative solution for these tests.
Comment 10 Mark Thomas 2014-03-06 15:59:36 UTC
This has been fixed in 7.0.x for 7.0.53 onwards and in 8.0.x for 8.0.4 onwards.
Comment 11 Francois-Xavier Bonnet 2014-03-14 16:51:29 UTC
Thanks. I just tested. Everything works as expected now.