Closed GoogleCodeExporter closed 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
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
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
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
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
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:
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
Original comment by evernat@free.fr
on 2 Jun 2012 at 9:19
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
that was quick ;) - thanks a lot - workaround works like a charm!
Original comment by bjomah...@gmail.com
on 4 Jun 2012 at 9:18
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
@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
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
Original issue reported on code.google.com by
bjomah...@gmail.com
on 8 May 2012 at 1:32