| Summary: | Add possibility to stop a ServerPushSession even if there are still Runnables to process | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [RT] RAP | Reporter: | Wolfgang Pedot <wolfgang.pedot> | ||||
| Component: | RWT | Assignee: | Project Inbox <rap-inbox> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | enhancement | ||||||
| Priority: | P3 | CC: | ivan | ||||
| Version: | 3.1 | ||||||
| Target Milestone: | 3.2 M6 | ||||||
| Hardware: | PC | ||||||
| OS: | Windows NT | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Wolfgang Pedot
Wolfgang, can you provide the stack trace of the exception? Is it possible to create a minimal RAP application to reproduce the exception? Is the stack trace similar: java.lang.NullPointerException at org.eclipse.rap.rwt.internal.serverpush.ServerPushManager.isSessionExpired(ServerPushManager.java:192) at org.eclipse.rap.rwt.internal.serverpush.ServerPushManager.isSessionExpired(ServerPushManager.java:187) at org.eclipse.rap.rwt.internal.serverpush.ServerPushManager.canReleaseBlockedRequest(ServerPushManager.java:156) at org.eclipse.rap.rwt.internal.serverpush.ServerPushManager.processRequest(ServerPushManager.java:140) at org.eclipse.rap.rwt.internal.serverpush.ServerPushServiceHandler.service(ServerPushServiceHandler.java:31) at org.eclipse.rap.rwt.engine.RWTServlet.handleValidRequest(RWTServlet.java:135) at org.eclipse.rap.rwt.engine.RWTServlet.handleRequest(RWTServlet.java:117) at org.eclipse.rap.rwt.engine.RWTServlet.doGet(RWTServlet.java:100) at javax.servlet.http.HttpServlet.service(HttpServlet.java:687) at javax.servlet.http.HttpServlet.service(HttpServlet.java:790) at org.eclipse.rap.rwt.osgi.internal.CutOffContextPathWrapper.service(CutOffContextPathWrapper.java:106) at org.eclipse.equinox.http.servlet.internal.HttpServiceRuntimeImpl$LegacyServlet.service(HttpServiceRuntimeImpl.java:1222) at org.eclipse.equinox.http.servlet.internal.registration.EndpointRegistration.service(EndpointRegistration.java:148) at org.eclipse.equinox.http.servlet.internal.servlet.ResponseStateHandler.processRequest(ResponseStateHandler.java:62) at org.eclipse.equinox.http.servlet.internal.context.DispatchTargets.doDispatch(DispatchTargets.java:131) at org.eclipse.equinox.http.servlet.internal.servlet.ProxyServlet.service(ProxyServlet.java:74) at javax.servlet.http.HttpServlet.service(HttpServlet.java:790) at org.eclipse.equinox.http.jetty.internal.HttpServerManager$InternalHttpServiceServlet.service(HttpServerManager.java:287) at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:845) at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:584) at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:224) at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1180) at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:512) at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:185) at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1112) at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141) at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:134) at org.eclipse.jetty.server.Server.handle(Server.java:534) at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:320) at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:251) at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:273) at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:95) at org.eclipse.jetty.io.SelectChannelEndPoint$2.run(SelectChannelEndPoint.java:93) at org.eclipse.jetty.util.thread.strategy.ExecuteProduceConsume.executeProduceConsume(ExecuteProduceConsume.java:303) at org.eclipse.jetty.util.thread.strategy.ExecuteProduceConsume.produceConsume(ExecuteProduceConsume.java:148) at org.eclipse.jetty.util.thread.strategy.ExecuteProduceConsume.run(ExecuteProduceConsume.java:136) at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:671) at org.eclipse.jetty.util.thread.QueuedThreadPool$2.run(QueuedThreadPool.java:589) at java.lang.Thread.run(Thread.java:745) Sorry that I did not include this in the original post:
java.lang.NullPointerException: The parameter 'uiSession' must not be null.
at org.eclipse.rap.rwt.internal.util.ParamCheck.notNull(ParamCheck.java:29)
at org.eclipse.rap.rwt.SingletonUtil.getUniqueInstance(SingletonUtil.java:73)
at org.eclipse.rap.rwt.SingletonUtil.getSessionInstance(SingletonUtil.java:58)
at org.eclipse.rap.rwt.internal.serverpush.ServerPushManager.getInstance(ServerPushManager.java:54)
at org.eclipse.rap.rwt.internal.serverpush.ServerPushServiceHandler.service(ServerPushServiceHandler.java:31)
at org.eclipse.rap.rwt.engine.RWTServlet.handleValidRequest(RWTServlet.java:135)
at org.eclipse.rap.rwt.engine.RWTServlet.handleRequest(RWTServlet.java:117)
at org.eclipse.rap.rwt.engine.RWTServlet.doGet(RWTServlet.java:100)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:687)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:790)
at org.eclipse.rap.rwt.osgi.internal.CutOffContextPathWrapper.service(CutOffContextPathWrapper.java:106)
at org.eclipse.equinox.http.servlet.internal.HttpServiceRuntimeImpl$LegacyServlet.service(HttpServiceRuntimeImpl.java:1217)
at org.eclipse.equinox.http.servlet.internal.registration.EndpointRegistration.service(EndpointRegistration.java:162)
at org.eclipse.equinox.http.servlet.internal.servlet.ResponseStateHandler.processRequest(ResponseStateHandler.java:63)
at org.eclipse.equinox.http.servlet.internal.context.DispatchTargets.doDispatch(DispatchTargets.java:98)
at org.eclipse.equinox.http.servlet.internal.HttpServiceRuntimeImpl.doDispatch(HttpServiceRuntimeImpl.java:372)
at org.eclipse.equinox.http.servlet.internal.servlet.ProxyServlet.service(ProxyServlet.java:70)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:790)
at org.eclipse.equinox.http.jetty.internal.HttpServerManager$InternalHttpServiceServlet.service(HttpServerManager.java:379)
at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:812)
at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:587)
at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:221)
at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143)
at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:97)
at at.finkzeit.zsw.server.jettycustomizer.internal.JettyStatistics.handle(JettyStatistics.java:137)
at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1129)
at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:515)
at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:185)
at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1061)
at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141)
at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:97)
at org.eclipse.jetty.server.Server.handle(Server.java:499)
at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:311)
at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:257)
at org.eclipse.jetty.io.AbstractConnection$2.run(AbstractConnection.java:544)
at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:635)
at org.eclipse.jetty.util.thread.QueuedThreadPool$3.run(QueuedThreadPool.java:555)
at java.lang.Thread.run(Thread.java:745)
The stack trace shows that you recieve a ServerPush request from the client after the UI session is terminated on the server. As a result, ServerPushManager singleton for this session can't be obtained. What I don't understand is how (why) the client sends this request. Could you provide a minimal application that demostrates the issue? Like I wrote ServerPushRenderer never renders deactivation of ServerPush because when we call ServerPushSession.stop there is still a runnable in the queue and I dont see another attempt to call it. The HTTP session gets terminated because of the redirect we do, if I disable that I see a very high rate of ServerPush-Requests because the client never stops sending them as it never receives the deactivation of ServerPush. ServerPushManager.processRequest does effectively nothing because isCallBackRequestBlocked and mustBlockCallBackRequest both return false. I am no heading into a meeting, maybe I can whip up a test afterwards. After further investigation I have found out that the issue I mention here is only responsible for a relatively small amount of those exceptions. It seems that a far greater portion comes from users just closing the browser-tab or navigating to another site without properly quitting the RAP application inside. Depending on the browser (FF does this a lot, Chrome and Edge appearantly not so much) I can produce this exception easily. Closing the tab leads to a direct shutdown of the uiSession and an immediate empty response. I have tried adding a static deactivation of ServerPush to LifeCycleServiceHandler.writeEmptyMessage and that seems to help but it does not really strike me as a very clean solution. It appears Chrome usually cancels all requests immediately when navigating away or closing the tab but FF (51.0.1 tested) sends at least one more ServerPush GET-Request most of the time. I have traced 324 of 355 exceptions in a sample-log to FF browsers, most of them version 51. It seems proper termination of ServerPush can be quite tricky.... Wolfgang, is it possiblr this issue to be related to this NG post -> https://www.eclipse.org/forums/index.php/t/1084101/? (In reply to Ivan Furnadjiev from comment #7) > Wolfgang, is it possiblr this issue to be related to this NG post -> > https://www.eclipse.org/forums/index.php/t/1084101/? I dont think so, I see the same errors when a ServerPush-GET request returns but I dont see why that should alter the behaviour of the browser. Setting a content-type of text/html in the ServerPush-responses got rid of the errors but does not do anything regarding this issue. I see two points here: 1.) ServerPush may not be terminated even though it has been stopped during application shutdown. I´ll try and get a small example together this afternoon. 2.) ServerPush is never terminated when a shutdown request is received from the browser (for example via onUnload) because the browser is not expected to send anything after that. I have traced the network traffic with FF and Chrome when closing a tab and FF sends another GET-request for ServerPush after receiving the final empty answer for the shutdown-request. Both cases can lead to the same exception on the server-side and while one may regard this as mostly a cosmetic issue I dont like hundrets of exceptions per day in my logfiles.... We can deactivate the server push on the client before the shutdown request is send. Add in Display#_onUnload:
---
_onUnload : function() {
this._document.removeEventListener( "windowresize", this._onResize, this );
this._document.removeEventListener( "keypress", this._onKeyPress, this );
this._server.removeEventListener( "send", this._onSend, this );
rwt.client.ServerPush.getInstance().setActive( false );
this._sendShutdown();
},
---
This should stop the GET-request for ServerPush after the session is terminated. What do you think?
That would fix point 2 nicely. As for 1.) I am having some troubles to reproduce the behaviour in the WorkbenchDemo, right now I only get the issue if I dont explicitly dispose the display. I am looking into that some more and will report once I have more information. Its really strange (at least for me): After further testing the (current) difference between my application and a patched demo is that in my application the ServerPushManager.FORCE_PUSH attribute is set in ServiceStore when ServerPushRenderer.render is called for the last time and that forces ServerPush to stay active. Since there are lots and lots of ServiceStore Objects created it is a little hard to track down the one the gets queried for that attribute when ServerPushRenderer.render is called for the last time. Any ideas how to reproduce that? Will continue tomorrow. Ok, just found the difference:
By using this piece of code in DemoWorkbench.createUI:
ServerPushSession pushSession = new ServerPushSession();
pushSession.start();
int result = PlatformUI.createAndRunWorkbench( display, worbenchAdvisor );
display.asyncExec( new Runnable() {
@Override
public void run() {
System.out.println( "RUNNING" );
}
} );
pushSession.stop();
I can get that FORCE_PUSH attribute in the "right" ServiceStore. That means that ServerPushRenderer will not render a deactivation and the browser will go into a request-loop once you terminate the workbench. Navigating to another page will then produce the famous exception.
This is not exactly the issue I reported originally but it is another boolean that will prevent ServerPush.deactivation from being rendered (just like ServerPushManager.hasRunnables) and its easier to reproduce.
There are essentially 3 conditions that must be met to render ServerPush deactivation (ServerPushManager.isServerPushActive, ServerPushManager.hasRunnables and ServerPushManager.forceServerPushForPendingRunnables) and the application can only directly control one of them (ServerPushManager.isServerPushActive)
Created attachment 266720 [details]
Full DemoWorkbench class used in test
Comment on attachment 266720 [details]
Full DemoWorkbench class used in test
I have now attached the full DemoWorkbench.java I used.
While trying to reproduce the issue in the demo I found that the Display was not disposed by the code in my application but only died because of onUnload or HTTP session termination. I dont really know why this was the case, that particular code is quite old (originally written for RAP 1.2 and adapted all the way up to 3.1) so I guess this is some old oversight.
The case that ServerPush does not render deactivation due to pending runnables does not seem to be reproducible any more after adding a call to display.dispose() because that flag is cleared on dispose.
The only one left is now the one reproducible with the file I attached. The test-code may seem a bit odd, in reality the path to display.asyncExec in my application is much more convolulted of course....
(In reply to Wolfgang Pedot from comment #14) > Comment on attachment 266720 [details] > Full DemoWorkbench class used in test > > I have now attached the full DemoWorkbench.java I used. > > While trying to reproduce the issue in the demo I found that the Display was > not disposed by the code in my application but only died because of onUnload > or HTTP session termination. I dont really know why this was the case, that > particular code is quite old (originally written for RAP 1.2 and adapted all > the way up to 3.1) so I guess this is some old oversight. > > The case that ServerPush does not render deactivation due to pending > runnables does not seem to be reproducible any more after adding a call to > display.dispose() because that flag is cleared on dispose. > > The only one left is now the one reproducible with the file I attached. The > test-code may seem a bit odd, in reality the path to display.asyncExec in my > application is much more convolulted of course.... Wolfgang, did you tried the proposed solution (Gerrit change attached to the bug)? Does it work for you? ... I'm talking about this change: https://git.eclipse.org/r/#/c/90590/ (In reply to Ivan Furnadjiev from comment #16) > ... I'm talking about this change: https://git.eclipse.org/r/#/c/90590/ Did I overlook that? Looks very promising, that seems to fix the last point on my list. (In reply to Wolfgang Pedot from comment #17) > (In reply to Ivan Furnadjiev from comment #16) > > ... I'm talking about this change: https://git.eclipse.org/r/#/c/90590/ > > Did I overlook that? > Looks very promising, that seems to fix the last point on my list. Actually, this fix solves both issue - when display is disposed by shutdown request or by code the ServerPush on the client will be deactivated. (In reply to Ivan Furnadjiev from comment #18) > > Actually, this fix solves both issue - when display is disposed by shutdown > request or by code the ServerPush on the client will be deactivated. I dont think so, the client always receives an emtpy response when sending the shutdown-request. See LifeCycleServiceHandler.processUIRequest. Yes... correct. I'll put the client solution back. Fixed with change https://git.eclipse.org/r/#/c/90590/ |