On 23/01/2014 14:42, r...@apache.org wrote: > Author: remm > Date: Thu Jan 23 14:42:21 2014 > New Revision: 1560702 > > URL: http://svn.apache.org/r1560702 > Log: > - Call onClose before actually closing anything (sending a close message > closes the endpoint).
-1 (veto) for a specification violation Section 2.1.5 requires that: "the implementation must attempt to send the websocket close frame prior to calling the onClose() method of the websocket endpoint." > - If onClose throws an exception, call onError. No objections. > - After using a blocking send, clear the client buffer in case it gets reused > (I have no idea why this is needed ...). This seems wrong (although I don't see the harm). I'd really expect the client to take care of that. Mark > > Modified: > > tomcat/trunk/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java > tomcat/trunk/java/org/apache/tomcat/websocket/WsSession.java > tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoEndpointBase.java > > Modified: > tomcat/trunk/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java > URL: > http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java?rev=1560702&r1=1560701&r2=1560702&view=diff > ============================================================================== > --- > tomcat/trunk/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java > (original) > +++ > tomcat/trunk/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java > Thu Jan 23 14:42:21 2014 > @@ -234,6 +234,7 @@ public abstract class WsRemoteEndpointIm > } else { > f2sh.get(timeout, TimeUnit.MILLISECONDS); > } > + payload.clear(); > } catch (InterruptedException | ExecutionException | > TimeoutException e) { > throw new IOException(e); > > Modified: tomcat/trunk/java/org/apache/tomcat/websocket/WsSession.java > URL: > http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/websocket/WsSession.java?rev=1560702&r1=1560701&r2=1560702&view=diff > ============================================================================== > --- tomcat/trunk/java/org/apache/tomcat/websocket/WsSession.java (original) > +++ tomcat/trunk/java/org/apache/tomcat/websocket/WsSession.java Thu Jan 23 > 14:42:21 2014 > @@ -45,6 +45,7 @@ import javax.websocket.WebSocketContaine > > import org.apache.juli.logging.Log; > import org.apache.juli.logging.LogFactory; > +import org.apache.tomcat.util.ExceptionUtils; > import org.apache.tomcat.util.res.StringManager; > > public class WsSession implements Session { > @@ -413,8 +414,8 @@ public class WsSession implements Sessio > > state = State.CLOSING; > > - sendCloseMessage(closeReasonMessage); > fireEndpointOnClose(closeReasonLocal); > + sendCloseMessage(closeReasonMessage); > > state = State.CLOSED; > } > @@ -436,8 +437,8 @@ public class WsSession implements Sessio > > synchronized (stateLock) { > if (state == State.OPEN) { > - sendCloseMessage(closeReason); > fireEndpointOnClose(closeReason); > + sendCloseMessage(closeReason); > state = State.CLOSED; > } > > @@ -455,6 +456,9 @@ public class WsSession implements Sessio > t.setContextClassLoader(applicationClassLoader); > try { > localEndpoint.onClose(this, closeReason); > + } catch (Throwable throwable) { > + ExceptionUtils.handleThrowable(throwable); > + localEndpoint.onError(this, throwable); > } finally { > t.setContextClassLoader(cl); > } > > Modified: > tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoEndpointBase.java > URL: > http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoEndpointBase.java?rev=1560702&r1=1560701&r2=1560702&view=diff > ============================================================================== > --- tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoEndpointBase.java > (original) > +++ tomcat/trunk/java/org/apache/tomcat/websocket/pojo/PojoEndpointBase.java > Thu Jan 23 14:42:21 2014 > @@ -64,14 +64,14 @@ public abstract class PojoEndpointBase e > log.error(sm.getString( > "pojoEndpointBase.onOpenFail", > pojo.getClass().getName()), e); > - handleOnOpenError(session, e); > + handleOnOpenOrCloseError(session, e); > return; > } catch (InvocationTargetException e) { > Throwable cause = e.getCause(); > - handleOnOpenError(session, cause); > + handleOnOpenOrCloseError(session, cause); > return; > } catch (Throwable t) { > - handleOnOpenError(session, t); > + handleOnOpenOrCloseError(session, t); > return; > } > } > @@ -83,7 +83,7 @@ public abstract class PojoEndpointBase e > } > > > - private void handleOnOpenError(Session session, Throwable t) { > + private void handleOnOpenOrCloseError(Session session, Throwable t) { > // If really fatal - re-throw > ExceptionUtils.handleThrowable(t); > > @@ -104,9 +104,9 @@ public abstract class PojoEndpointBase e > methodMapping.getOnClose().invoke(pojo, > methodMapping.getOnCloseArgs(pathParameters, > session, closeReason)); > } catch (Throwable t) { > - ExceptionUtils.handleThrowable(t); > log.error(sm.getString("pojoEndpointBase.onCloseFail", > pojo.getClass().getName()), t); > + handleOnOpenOrCloseError(session, t); > } > } > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
