Hi, 2017-04-26 17:43 GMT+03:00 Mark Thomas <ma...@apache.org>: > > 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.
The fixes for all comments are available in the PR. If there are no other comments I'm going to commit this functionality to Tomcat 9. Thanks, Violeta > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org >
