search

me0wster / javamelody

Automatically exported from code.google.com/p/javamelody
0 stars 0 forks source link

AsyncContext.getResponse().getWriter() throws IllegalStateException #217

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
I have a problem with an asynchronous servlet and the servlet filter of melody.

1. I built a web application with a asynchronous servlet (servlet 3.0 api). The 
Servlet has an own thread writing to the response stream of an asynchronous 
context...

2. i installed melody
3. the filter configured in the web.xml (with supportAsync set to true) writes 
or flushes the response stream - so this get into an illegal state.

4. After that, you can't call a writer method of the response without getting 
this exception:

java.lang.IllegalStateException: getOutputStream() has already been called for 
this response
    at org.apache.catalina.connector.Response.getWriter(Response.java:633)
    at org.apache.catalina.connector.ResponseFacade.getWriter(ResponseFacade.java:214)
    at myapplication.AsyncServlet.sendMessage(AsyncServlet.java:---)

I'm using JDK 1.6.27 on a tomcat 7.0.23 (win7).

In the class MonitoringFilter in line 198 i see the following:
wrappedResponse.flushBuffer();

which will write some stuff to the response stream...

I think there is somethin' wrong..

Original issue reported on code.google.com by bjomah...@gmail.com on 8 May 2012 at 1:32

GoogleCodeExporter commented 9 years ago 
I also cannot imagine why the filter needs to flush the buffer - but until now 
I was only concerned about performance regarding this (see 
http://code.google.com/p/javamelody/issues/detail?id=186 )

Original comment by mineral_...@gmx.de on 8 May 2012 at 4:00

GoogleCodeExporter commented 9 years ago 
wrappedResponse.flushBuffer() is there because otherwise some cases would not 
work (for example a simple JSP writing text and called directly from the 
browser, as tested with Tomcat 6.0.29).

I will come back to async later.

First, the fact is that in tomcat and others, a JSP uses a "writer" and not a 
binary "outputStream". In order to count the response size, the 
MonitoringFilter [1] wraps the response in a CounterServletResponseWrapper [2] 
to use a CounterResponseStream [3].
The CounterServletResponseWrapper extends FilterServletResponseWrapper [4], and 
its getWriter() method [5] wraps the (CounterResponse)Stream in a PrintWriter 
and in an OutputStreamWriter.
This OutputStreamWriter has a buffer and this buffer needs to be flushed in a 
stream so that its content is not lost.
And so there is wrappedResponse.flushBuffer().

(That said, I am not so sure that the 
FilterServletResponseWrapper.flushBuffer() method needs to be called when the 
wrappedResponse has just an outputStream and no writer.)

[1] 
http://code.google.com/p/javamelody/source/browse/trunk/javamelody-core/src/main
/java/net/bull/javamelody/MonitoringFilter.java
[2] 
http://code.google.com/p/javamelody/source/browse/trunk/javamelody-core/src/main
/java/net/bull/javamelody/CounterServletResponseWrapper.java
[3] 
http://code.google.com/p/javamelody/source/browse/trunk/javamelody-core/src/main
/java/net/bull/javamelody/CounterResponseStream.java
[4] 
http://code.google.com/p/javamelody/source/browse/trunk/javamelody-core/src/main
/java/net/bull/javamelody/FilterServletResponseWrapper.java
[5] 
http://code.google.com/p/javamelody/source/browse/trunk/javamelody-core/src/main
/java/net/bull/javamelody/FilterServletResponseWrapper.java#117

Original comment by evernat@free.fr on 28 May 2012 at 2:57

GoogleCodeExporter commented 9 years ago 
I have not reproduced the issue about async.
I have added the current javamelody jar file, the jrobin jar file and I have 
modified the web.xml file in the "examples" webapp of Tomcat 7.0.27.
You can download this modified "examples" webapp from:
http://javamelody.googlecode.com/files/examples.zip

And I have started Tomcat 7.0.27 with this modified "examples" webapp in the 
"webapps" directory of Tomcat.
The async JSP are async0, async1, async2, async3 and stockticker. They can be 
tested from:
http://localhost:8080/examples/jsp/
All 5 async jsp work fine.

The monitoring report shows the http statistics of the async jsp correctly, and 
the mean size of the responses is 0 KB:
http://localhost:8080/examples/monitoring

Original comment by evernat@free.fr on 28 May 2012 at 4:55

GoogleCodeExporter commented 9 years ago 
So is it possible for you to upload in this issue an example to reproduce the 
problem with async?
Thanks

Original comment by evernat@free.fr on 28 May 2012 at 4:58

GoogleCodeExporter commented 9 years ago 
I think I have to test again... i used a similar example of this one:
https://github.com/flowersinthesand/jquery-socket/tree/master/samples/servlet-st
ream

as for that, I would expect, that the async2 example fails also. As I tested 
the only thing i changed was the javamelody filter. After switching it on or 
off, I saw that was the failing trigger - and I only gave a short look at the 
code... 

I'll reproduce this, test also the example and then, if I'll fail again I'll 
post a failing example. 

Thanks for your support!

Original comment by bjomah...@gmail.com on 29 May 2012 at 6:14

GoogleCodeExporter commented 9 years ago 
Okay - i made a project - actually the example i mentioned above with stripped 
functionality for servlet 3.0 only. The pom only has to have the dependency to 
javamelody and the async servlet is not working anymore.

Original comment by bjomah...@gmail.com on 1 Jun 2012 at 1:51

Attachments:

GoogleCodeExporter commented 9 years ago 
The code of the servlet. The call of getWriter in the Thread leads to the 
mentioned IllegalStateException.

hth - and i hope you'll fix that or have an idea of what I did wrong ;) because 
javamelody would be a very nice to have for our project.

TIA
Björn

package flowersinthesand.example;

import com.google.gson.Gson;

import javax.servlet.*;
import javax.servlet.annotation.WebServlet;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.io.IOException;
import java.io.PrintWriter;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.UUID;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.LinkedBlockingQueue;

@WebServlet(urlPatterns = "/chat", asyncSupported = true)
public class HttpChatServlet extends HttpServlet {

    /* Common */
    private BlockingQueue<String> messages = new LinkedBlockingQueue<String>();
    private Thread notifier = new Thread(new Runnable() {
        public void run() {
            boolean done = false;
            while (!done) {
                try {
                    String message = messages.take();
                    for (AsyncContext asyncContext : asyncContexts.values()) {
                        try {
                            // Message
                            PrintWriter writer = asyncContext.getResponse().getWriter();
                            writer.print(message.length());
                            writer.print(";");
                            writer.print(message);
                            writer.print(";");
                            writer.flush();
                        } catch (Exception e) {
                            e.printStackTrace();  //<--- here we go when calling getWriter above!
              asyncContexts.values().remove(asyncContext);
                        }
                    }
                } catch (InterruptedException e) {
          e.printStackTrace();
                    done = true;
                }
            }
        }
    });

    @Override
    public void init(ServletConfig config) throws ServletException {
        super.init(config);
        notifier.start();
    }

    @Override
    public void destroy() {
        messages.clear();
        asyncContexts.clear();
        notifier.interrupt();
    }

    /* HTTP Streaming powered by Servlet 3.0 */
    private Map<String, AsyncContext> asyncContexts = new ConcurrentHashMap<String, AsyncContext>();

    // GET method is used to open stream
    @Override
    protected void doGet(HttpServletRequest request, HttpServletResponse response)
            throws ServletException, IOException {
        // Wrong access
        if ("websocket".equalsIgnoreCase(request.getHeader("Upgrade"))) {
            response.sendError(HttpServletResponse.SC_NOT_IMPLEMENTED);
            return;
        }

        response.setCharacterEncoding("utf-8");

        // Content-Type header
        response.setContentType("text/plain");

        // Access-Control-Allow-Origin header
        response.setHeader("Access-Control-Allow-Origin", "*");

        PrintWriter writer = response.getWriter();

        // Id
        final String id = UUID.randomUUID().toString();
        writer.print(id);
        writer.print(';');

        // Padding
        for (int i = 0; i < 1024; i++) {
            writer.print(' ');
        }
        writer.print(';');
        writer.flush();

        final AsyncContext ac = request.startAsync();
        ac.setTimeout(5 * 60 * 1000);
        ac.addListener(new AsyncListener() {
            public void onComplete(AsyncEvent event) throws IOException {
                asyncContexts.remove(id);
            }

            public void onTimeout(AsyncEvent event) throws IOException {
                asyncContexts.remove(id);
            }

            public void onError(AsyncEvent event) throws IOException {
                asyncContexts.remove(id);
            }

            public void onStartAsync(AsyncEvent event) throws IOException {

            }
        });
        asyncContexts.put(id, ac);
    }

    // POST method is used to handle data sent by user through the stream
    @Override
    protected void doPost(HttpServletRequest request, HttpServletResponse response)
            throws ServletException, IOException {
        request.setCharacterEncoding("utf-8");

        if ("close".equals(request.getParameter("metadata.type"))) {
            AsyncContext ac = asyncContexts.get(request.getParameter("metadata.id"));
            if (ac != null) {
                ac.complete();
            }

            return;
        }

        // Handles data sent from a client
        Map<String, String> data = new LinkedHashMap<String, String>();
        data.put("username", request.getParameter("username"));
        data.put("message", request.getParameter("message"));

        try {
            messages.put(new Gson().toJson(data));
        } catch (InterruptedException e) {
            throw new IOException(e);
        }
    }

}

Original comment by bjomah...@gmail.com on 1 Jun 2012 at 1:55

GoogleCodeExporter commented 9 years ago

Original comment by evernat@free.fr on 2 Jun 2012 at 9:19

GoogleCodeExporter commented 9 years ago 
It is caused by the use of response.getWriter() in your doGet method and then 
by the use of AsyncContext.getResponse().getWriter() in your async thread.
The problem is that the "response" object in doGet is in fact an instance of a 
javamelody wrapper, but AsyncContext.getResponse() returns the Tomcat instance 
and not the wrapper.
A workaround would be to call "request.startAsync(request, response)" in doGet, 
instead of "request.startAsync()", so that AsyncContext.getResponse() returns 
the wrapper.

But anyway it should be fixed now without needing to change your code. The fix 
is in trunk (revision 2859 prepared by revision 2858) and it is ready for the 
next release (1.38).
The important line is at line 80 there:
https://code.google.com/p/javamelody/source/browse/trunk/javamelody-core/src/mai
n/java/net/bull/javamelody/JspWrapper.java#80

I have made a new build which you can test. It is available at:
http://javamelody.googlecode.com/files/javamelody-20120602.jar

Original comment by evernat@free.fr on 2 Jun 2012 at 10:08

GoogleCodeExporter commented 9 years ago 
that was quick ;) - thanks a lot - workaround works like a charm!

Original comment by bjomah...@gmail.com on 4 Jun 2012 at 9:18

GoogleCodeExporter commented 9 years ago 
getting same error even after I upgrade to latest. tried 1.38 and 1.49. But the 
scenario in my case is when using hot deploy/startup using maven. I've tried 
with both tomcat and jetty maven plugins and same issue happening. But when i 
deploy as separate war app onto tomcat, i've not come across this issue even 
for the version prior to 1.38.
Any hints to a workaround.

Original comment by eswaru...@gmail.com on 27 Jul 2014 at 10:02

GoogleCodeExporter commented 9 years ago 
@eswarup69
Can you provide a simple maven project in a zip file and steps to reproduce?
Thanks

Original comment by evernat@free.fr on 28 Jul 2014 at 7:52

GoogleCodeExporter commented 9 years ago 
Hey just fyi my colleague got the same error even with a separate install of 
tomcat on mac mavericks tomcat v7.0.54. I'm still trying to see if I can get 
zip file to reproduce it.

Original comment by eswaru...@gmail.com on 29 Jul 2014 at 3:45