Jetty 9.3 and Atmosphere do not play nicely together

[Artur-]

Tested using the chat sample and Atmosphere 2.3.2

12:58:45.807 ERROR [qtp1041905665-36] o.a.c.AsynchronousProcessor [AsynchronousProcessor.java:315] Interceptor Heartbeat Interceptor Support crashed. Processing will continue with other interceptor.
java.lang.NullPointerException: null
    at org.eclipse.jetty.server.Request.getContentType(Request.java:658) ~[na:na]
    at org.eclipse.jetty.server.Request.getCharacterEncoding(Request.java:608) ~[na:na]
    at org.atmosphere.cpr.AtmosphereRequest.getCharacterEncoding(AtmosphereRequest.java:1089) ~[atmosphere-runtime-2.3.2.jar:2.3.2]
    at org.atmosphere.cpr.AtmosphereRequest.getCharacterEncoding(AtmosphereRequest.java:1089) ~[atmosphere-runtime-2.3.2.jar:2.3.2]
    at org.atmosphere.util.IOUtils.readEntirelyAsByte(IOUtils.java:233) ~[atmosphere-runtime-2.3.2.jar:2.3.2]
    at org.atmosphere.interceptor.HeartbeatInterceptor.inspect(HeartbeatInterceptor.java:191) ~[atmosphere-runtime-2.3.2.jar:2.3.2]
    at org.atmosphere.cpr.AsynchronousProcessor.invokeInterceptors(AsynchronousProcessor.java:313) [atmosphere-runtime-2.3.2.jar:2.3.2]
    at org.atmosphere.cpr.AsynchronousProcessor.action(AsynchronousProcessor.java:174) [atmosphere-runtime-2.3.2.jar:2.3.2]
    at org.atmosphere.cpr.AsynchronousProcessor.suspended(AsynchronousProcessor.java:108) [atmosphere-runtime-2.3.2.jar:2.3.2]
    at org.atmosphere.container.Jetty9AsyncSupportWithWebSocket.service(Jetty9AsyncSupportWithWebSocket.java:180) [atmosphere-runtime-2.3.2.jar:2.3.2]

Similar problems reported by a Vaadin user at https://vaadin.com/forum#!/thread/10309898

Jun 16, 2015 12:17:42 PM org.atmosphere.cpr.AtmosphereResourceImpl notifyListeners
WARNING: Exception during suspend() operation java.lang.NullPointerException
Jun 16, 2015 12:17:42 PM com.vaadin.server.communication.PushHandler onThrowable
SEVERE: Exception in push connection
java.lang.NullPointerException
    at org.eclipse.jetty.server.Request.extractQueryParameters(Request.java:371)
    at org.eclipse.jetty.server.Request.extractParameters(Request.java:357)
    at org.eclipse.jetty.server.Request.getParameter(Request.java:989)
    at javax.servlet.ServletRequestWrapper.getParameter(ServletRequestWrapper.java:194)
    at javax.servlet.ServletRequestWrapper.getParameter(ServletRequestWrapper.java:194)

Jetty 9.3 issue or Atmosphere issue?

[jfarcand]

Sound like in 9.3 Jetty's Native WebSocket Implementation with websocket is recycling the HttpServletRequest hence all field are nuill

[jfarcand]

Workaround is to enable jsr 356 https://github.com/Atmosphere/atmosphere/wiki/Forcing-JSR-356-for-WebSocket-Application

[joakime]

Can you describe when, in the lifecycle of the websocket, you are accessing these fields from the HttpServletRequest? The request object does get recycled eventually, but it should still be valid during handshake / onopen (which is the safest point to access this information).

The stacktrace above doesn't indicate access to the Request object from within any of the websocket layers, but rather from the servlet itself (which is very strange to see)

[joakime]

From the longer stacktrace at the vaadin side, the problem occurs through this path ...

• Servlet30CometSupport.service()
• AsynchronousProcessor.suspend()
• AsynchronousProcessor.action()
• AsynchronousProcessor.invokeInterceptors()
• OnDisconnectInterceptor.inspect()
• AsynchronousProcessor.completeLifecycle()
• PushHandler.onDisconnect()

and then you go and attempt to create a VaadinSession during this disconnect flow, and part of that logic is to extract parameters from the request object.

Websocket isn't involved in that stacktrace, you have a disconnected/disconnecting client and are being notified of such, however you then attempt to create a VaadinSession during this state.

You shouldn't be attempting to access the request object during this disconnect state.

[jfarcand]

@joakime The issue arises because the original HttpServletRequest passed to the WebSocketCreator via the acceptWebSocket

            public boolean acceptWebSocket(final HttpServletRequest request, final HttpServletResponse response) throws IOException {
                setCreator(new WebSocketCreator() {
                    // @Override 9.1.x
                    public Object createWebSocket(ServletUpgradeRequest req, ServletUpgradeResponse resp) {
                        req.getExtensions().clear();

                        if (!webSocketProcessor.handshake(request)) {
                            try {
                                response.sendError(HttpServletResponse.SC_FORBIDDEN, "WebSocket requests rejected.");
                            } catch (IOException e) {
                                logger.trace("", e);
                            }
                            return null;
                        }
                        return new Jetty9WebSocketHandler(request, config.framework(), webSocketProcessor);
                    }
                });

                return super.acceptWebSocket(request, response);
            }

See

is getting recycled/reset in 9.3.0 to null after executing .

    @Override
    public void onWebSocketConnect(Session session) {
        logger.trace("WebSocket.onOpen.");
        webSocket = new Jetty9WebSocket(session, framework.getAtmosphereConfig());

See
Atmosphere uses that request to get access to the 'HttpSession', attributes, headers, etc.

One workaround in Atmosphere consists of loading the content of that request in memory but that's kind of bad because it's a copy of the original request object instead of the object itself. But that's a possible workaround for now.

[joakime]

Note: the ServletUpgradeRequest, part of the WebSocketCreator method calls, is durable, and should be an immutable copy of the values of the original HttpServletRequest object.

[joakime]

Referencing in-line comment on commit here too.

The originalRequest(Session session) fetch of the HttpServletRequest via reflection is bad style.
You cannot access the original HttpServletRequest at any point outside of the WebSocketCreator.createWebSocket() call, all other access is unsupported.

Use the ServletUpgradeRequest and its methods only, that's durable and long lasting (for the lifecycle of the WebSocket).

Note: when WebSocket over HTTP/2 hits, this is going to be very important.

If there are some methods missing in ServletUpgradeRequest that you need, ask for them, we'll expose them.

Also, there's a method on ServletUpgradeRequest to access the original HttpServletRequest if you need it, but (again) the HttpServletRequest only valid during the WebSocketCreator.createWebSocket() call.

Once the connection is upgraded, then the HttpServletRequest is recycled, and all of the concepts of servlets are null and void. (even lifecycle listeners)

[jfarcand]

@joakime I clone the ServletUpgradeRequest internal state with the current fixe to indeed prevent loosing the request's information. I did try to use the ServletUpgradeRequest but I was getting all sort of UnsupportedException. One improvemen I can made is to wrap the ServletUpgradeRequest inside an 'HttpServletRequest` or use a Proxy to delegate to that object.

[benstpierre]

Is this going to be fixed? It seems to me that Jetty stable from now on will be using this new HTTP2 architecture that seems to be causing this issue.

[jfarcand]

@benstpierre It is fixed in 2.3.3-SNAPSHOT

[Artur-]

There is still at least one NPE issue remaining, created #2154 to track that

[joakime]

Issue #2154 has been closed due to an old JVM and its associated bugs.

[jyoti-sungard]

I am facing an issue in getting previsouly created HttpSession with Atmosphere request.
Please help me! I am stuck now.

It was working fine with jetty 9.2.9 but when I upgrade jetty to 9.3.5 , session info object created previously after login is now null in the onRequest of MessagingEndPoint of atmosphere

The main cause of the issue is the following change in Jetty9websocketHandler in the Atmosphere-runtime library

/**
* #1998
* The Original Jetty Request will be recycled, hence we must loads its content in memory. We can't do that before
* as it break Jetty 9.3.0 upgrade process.
*
* This is a performance regression from 9.2 as we need to clone again the request. 9.3.0+ should use jsr 356!
*/
if (jetty93Up) {
HttpServletRequest r = originalRequest(session);
if (r != null) {
// We close except the session which we can still reach.
request = AtmosphereRequest.cloneRequest(r, true, true, false, framework.getAtmosphereConfig().getInitParameter(PROPERTY_SESSION_CREATE, true));
} else {
// Bad Bad Bad
request = AtmosphereRequest.cloneRequest(r, true, true, false, framework.getAtmosphereConfig().getInitParameter(PROPERTY_SESSION_CREATE, true));
}
}

[joakime]

Note, the comment "9.3.0+ should use jsr 356" is pointless. Jetty offers both native websocket (worlds better then JSR356) and JSR356 support (v1.0 currently, 1.1 scheduled for Jetty 9.4).

Both options have the same issue with HttpSession, the container is free to recycle the HttpServletRequest object once the upgrade is complete.

The lifecycle of HttpServletRequest and HttpSession have no application when you have upgraded out of a HTTP request and into a WebSocket lifecycle. This is a problem for many libraries, not just yours. Even CDI has no solution for this lifecycle difference. Not even JSR356 has addressed this issue (its an open bug/issue on the JSR356 issue tracker). The HttpSession is also free to expire / idle timeout (and get recycled) as there's no activity on the HttpServletRequest, or HttpServletResponse while you are in WebSocket.

There is some hope that the Servlet 4.x effort (currently underway) will properly declare how the 2 different lifecycles should behave. (Note: this will likely have some big impacts to your code once its properly defined)

[joakime]

This is very bad style, and is the cause of your issues.

https://github.com/Atmosphere/atmosphere/blob/atmosphere-3.0.x/modules/cpr/src/main/java/org/atmosphere/container/Jetty9WebSocketHandler.java#L131-L147

    private HttpServletRequest originalRequest(Session session) {
        try {
            // Oh boy...
            ServletUpgradeRequest request = (ServletUpgradeRequest) session.getUpgradeRequest();
            Field[] fields = ServletUpgradeRequest.class.getDeclaredFields();
            for (Field f : fields) {
                f.setAccessible(true);
                Object o = f.get(request);
                if (o instanceof HttpServletRequest) {
                    return HttpServletRequest.class.cast(o);
                }
            }
        } catch (Exception ex) {
            logger.error("", ex);
        }
        return null;
    }

Do not copy the HttpServletRequest object out of the ServletUpgradeRequest, it is not durable in the context of a connected WebSocket lifecycle. (The HttpServletRequest is only available during the upgrade process, and once that is done its gone)

If what you want is the HttpSession, use the ServletUpgradeRequest.getSesssion(), which is copied out of the HttpServletRequest during WebSocket upgrade.

But note that the HttpSession it returns is also subject to the lack of lifecycle definition in the various specs.

For example:

• That HttpSession should be considered a snapshot / read-only view into the HttpSession at the time of WebSocket upgrade.
• The content of that HttpSession is not updated if the HttpSession is updated elsewhere (in a different request)
• That HttpSession can time out easily, as there's no HttpServletRequest or HttpServletResponse active during WebSocket that can keep the HttpSession alive / updated.
• As there's no active HttpServletRequest or HttpServletReponse context alive during WebSocket, changes to HttpSession values are not persisted, and HttpSession state changes (eg: .invalidate()) are also not persisted.

There are not insurmountable issues for a library using WebSocket, many libraries built on top of Java WebSocket / JSR356 have adapted to this reality of ill defined specs and lifecycles just fine, atmosphere can too.

[jfarcand]

@joakime Well, the Jetty API changed many times and that's the reason why I needed to use reflection here. I do agree this is quite ugly, but supporting 9.0/9.1/9.2 and 9.3 is a challenge with this unique code base. Now fixing the session should be straightforward...and the HttpServletRequest is later cloned in the code (it's content copied during the handshake) and the object never used after. So yes it's ugly, but we don't keep a reference to it after the handshake.

[joakime]

The support for 9.0 is suspect, that was a transition release between Servlet 3.0 and Servlet 3.1, probably safe to drop support for 9.0. Depending on what specific version of 9.0.x you might either have a Servlet 3.0 support level, or a Servlet 3.1 beta/draft support level.

As for popularity, we have statistics from maven central and download.eclipse.org to work with...

  7.x =  8.95%  EOL (near steady state)
  8.x = 28.51%  EOL (declining *very slowly*)
9.0.x =  2.46%      (near steady state)
9.1.x =  2.57%      (declining slowly)
9.2.x = 42.34%      (declining)
9.3.x = 15.17%      (climbing)

This shows you that Jetty 9.0.x and 9.1.x interest are practically statistical noise at this point and likely safe to start deprecating in your own releases.

Note: Jetty 9.4.x is due out this summer, and Jetty 10.x has already been started (its the master branch in git ATM)

[jyoti-sungard]

@jfarcand - Are you going to fix this as you said now fixing the session should be straightforward?
Otherwise, I need to build it locally. Currently I am using atmosphere-runtime 2.3.5.

Thanks In Advance!

[jfarcand]

@jyoti-sungard I don't have the cycle to do that this month...so contribution welcomed :-)

[jyoti-sungard]

@jfarcand - I need to pass copySession as true in theJetty9WebSocketHandler.java to get the previously created session. Do you guys have any concern as the comment says "This is a performance regression from 9.2 as we need to clone again the request. 9.3.0+ ".

https://github.com/Atmosphere/atmosphere/blob/master/modules/cpr/src/main/java/org/atmosphere/container/Jetty9WebSocketHandler.java

[jfarcand]

@jyoti-sungard That should be fine to re-use the session, event if we copy the request's state.

[jyoti-sungard]

Hi,
i'm trying to integrate the latest spring (4.2.2.RELEASE) and latest atmosphere(2.4.4) on jetty 9.3.5.
Are those frameworks versions supposed to work together?

Here the error code which seems to be related to a bad URI:

(xyz is the pubsub topic i entered; spring-atmosphere is the application name)

java.lang.IllegalStateException: Could not get HttpServletRequest URI: Expected hostname at index 5: ws://:0/messaging/realtime?topic=xyz

org.springframework.http.server.ServletServerHttpRequest.getURI(ServletServerHttpRequest.java:98)\r\n\tat org.springframework.web.util.UriComponentsBuilder.fromHttpRequest(UriComponentsBuilder.java:282)\r\n\tat org.springframework.web.util.WebUtils.isSameOrigin(WebUtils.java:811)\r\n\tat org.springframework.web.cors.DefaultCorsProcessor.processRequest(DefaultCorsProcessor.java:71)\r\n\tat org.springframework.web.servlet.handler.AbstractHandlerMapping$CorsInterceptor.preHandle(AbstractHandlerMapping.java:503)\r\n\tat org.springframework.web.servlet.HandlerExecutionChain.applyPreHandle(HandlerExecutionChain.java:134)\r\n\tat org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:954)\r\n\tat org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:893)\r\n\tat org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:970)\r\n\tat org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:861)\r\n\tat javax.servlet.http.HttpServlet.service(HttpServlet.java:687)\r\n\tat org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:846)\r\n\tat javax.servlet.http.HttpServlet.service(HttpServlet.java:790)\r\n\tat org.atmosphere.util.AtmosphereFilterChain.doFilter(AtmosphereFilterChain.java:135)\r\n\tat org.atmosphere.util.AtmosphereFilterChain.invokeFilterChain(AtmosphereFilterChain.java:96)\r\n\tat org.atmosphere.handler.ReflectorServletProcessor$FilterChainServletWrapper.service(ReflectorServletProcessor.java:333)\r\n\tat org.atmosphere.handler.ReflectorServletProcessor.onRequest(ReflectorServletProcessor.java:171)\r\n\tat org.atmosphere.cpr.AsynchronousProcessor.action(AsynchronousProcessor.java:223)\r\n\tat org.atmosphere.cpr.AsynchronousProcessor.suspended(AsynchronousProcessor.java:115)\r\n\tat org.atmosphere.container.Servlet30CometSupport.service(Servlet30CometSupport.java:68)

Thanks!

[joakime]

The websocket uri you are using ws://:0/messaging/realtime?topic=xyz is an invalid URI.

The host:port part is missing its host, and is using an invalid port number.