On 25/04/17 11:47, Violeta Georgieva wrote: <snip/>
> Thanks for the review. > Changes for all comments are applied to the PR. > Can you take a look? Sure. A few more comments but nothing serious. Unless the fixes for any of these require large changes to the patch I'd be +1 on applying the patch with these fixes. I'd be fine with the patch being committed without the minor issues fixed as long as they were addressed later. Mark Moderate - If another thread calls suspend() after the call to close() it looks like there could be an issue. Is another state - CLOSING - required? - On the client READ means a read is progress and READY means data has been read and is being processed. On the server the meanings are reversed. - A couple of lines have trailing whitespace (only moderate because it will break the CI system) Minor - The Javadoc for the state diagram would be clearer with separate lines for each transition rather than some lines being bi-directional - Can WsFrameClient.processSocketRead() be simplified? The try/catch block that sets read state to READY looks to be unnecessary. The code paths all appear to lead to close - and that sets the read state anyway. --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
