[Bug 54746] New: Cannot obtain information from Session in onClose method

[b***@apache.org]

https://issues.apache.org/bugzilla/show_bug.cgi?id=54746

Bug ID: 54746
Summary: Cannot obtain information from Session in onClose
method
Product: Tomcat 8
Version: trunk
Hardware: All
OS: All
Status: NEW
Severity: major
Priority: P2
Component: Catalina
Assignee: ***@tomcat.apache.org
Reporter: ***@nicholaswilliams.net
Classification: Unclassified

See the exception below. I attempted to call getUserProperties() in a method
annotated @OnClose. Here is what the WebSocket spec says about this:

[quote]Once the session is closed, it is no longer valid for use by
applications. Calling any of its methods (with the exception of the close()
methods) once the session has been closed will result in an {@link
java.lang.IllegalStateException} being thrown. Developers should retrieve any
information from the session during the {@link Endpoint#onClose }
method.[/quote]

This clearly indicates that it is valid to "retrieve any information from the
session" DURING the call to onClose. Therefore, this exception should not have
been thrown.

SEVERE: Failed to call onError method of POJO end point for POJO of type
[com.wrox.chat.ChatEndpoint]
java.lang.reflect.InvocationTargetException
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:487)
at
org.apache.tomcat.websocket.pojo.PojoEndpointBase.onError(PojoEndpointBase.java:127)
at
org.apache.tomcat.websocket.server.WsHttpUpgradeHandler.onError(WsHttpUpgradeHandler.java:139)
at
org.apache.tomcat.websocket.server.WsHttpUpgradeHandler.access$300(WsHttpUpgradeHandler.java:42)
at
org.apache.tomcat.websocket.server.WsHttpUpgradeHandler$WsReadListener.onError(WsHttpUpgradeHandler.java:197)
at
org.apache.tomcat.websocket.server.WsHttpUpgradeHandler$WsReadListener.onDataAvailable(WsHttpUpgradeHandler.java:183)
at
org.apache.coyote.http11.upgrade.AbstractServletInputStream.onDataAvailable(AbstractServletInputStream.java:169)
at
org.apache.coyote.http11.upgrade.AbstractProcessor.upgradeDispatch(AbstractProcessor.java:94)
at
org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(AbstractProtocol.java:618)
at
org.apache.coyote.http11.Http11NioProtocol$Http11ConnectionHandler.process(Http11NioProtocol.java:223)
at
org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1581)
at
org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.run(NioEndpoint.java:1537)
at
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1110)
at
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:603)
at java.lang.Thread.run(Thread.java:722)
Caused by: java.lang.IllegalStateException: The WebSocket session has been
closed and no method (apart from close()) may be called on a closed session
at org.apache.tomcat.websocket.WsSession.checkState(WsSession.java:504)
at
org.apache.tomcat.websocket.WsSession.getUserProperties(WsSession.java:455)
at com.wrox.chat.ChatEndpoint.onError(ChatEndpoint.java:154)
... 18 more

[b***@apache.org]

https://issues.apache.org/bugzilla/show_bug.cgi?id=54746

--- Comment #1 from Nick Williams <***@nicholaswilliams.net> ---
More information here: http://java.net/jira/browse/WEBSOCKET_SPEC-174

[b***@apache.org]

https://issues.apache.org/bugzilla/show_bug.cgi?id=54746

Mark Thomas <***@apache.org> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |INVALID

--- Comment #2 from Mark Thomas <***@apache.org> ---
That is error handling rather than a normal close. If you are going to try and
access the session from an error handler you need to take account of the fact
that the container may already have closed the connection in response to the
error or, equally, that the error could be that the connection was dropped by
the client / network / take your pick.

[b***@apache.org]

https://issues.apache.org/bugzilla/show_bug.cgi?id=54746

Nick Williams <***@nicholaswilliams.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|RESOLVED |REOPENED
Resolution|INVALID |---

--- Comment #3 from Nick Williams <***@nicholaswilliams.net> ---
I understand your point that this happens in onError and not onClose (my bad
for mis-reporting), but I think that highlights a problem with the spec. I have
commented on http://java.net/jira/browse/WEBSOCKET_SPEC-174 to request further
clarification, and request that this Bugzilla remain open until the spec issue
is further clarified.

[b***@apache.org]

https://issues.apache.org/bugzilla/show_bug.cgi?id=54746

Nick Williams <***@nicholaswilliams.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Summary|Cannot obtain information |Cannot obtain idempotent
|from Session in onClose |information from Session in
|method |onError method after
| |session closed

[b***@apache.org]

https://issues.apache.org/bugzilla/show_bug.cgi?id=54746

Mark Thomas <***@apache.org> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|REOPENED |RESOLVED
Resolution|--- |WONTFIX

--- Comment #4 from Mark Thomas <***@apache.org> ---
The final WebSocket spec is very clear on the required behaviour and this is
what Tomcat implements.

I've looked through the code and the only time I can find where
Endpoint.onError() is called after the session has been closed is if for the
following sequence of events:
- Server receives close message
- server fires session.onClose()
- session is marked as closed
- server echos close message back to the client
- the sending of that message fails

This is easily detected via the onOpen() method of the session and isn't -
given that as far as the app is concerned it was a normal shutdown - something
I am too concerned about. It could be argued that it would be reasonable in
this case to just swallow the error. It would be useful to get some clarity on
this from the EG but I don't see it as a bug issue.

I also took a look at how things like network errors are handled. In that case
onClose() is triggered with a suitable close code.

If you have a test case (other than the one above) that demonstrates onClose()
being called before onError() I'd be happy to take look. Just re-open this and
provide some steps to reproduce.