On Wed, May 15, 2019 at 2:25 PM Mark Thomas <ma...@apache.org> wrote:
> On 15/05/2019 12:56, r...@apache.org wrote: > > This is an automated email from the ASF dual-hosted git repository. > > > > remm pushed a commit to branch master > > in repository https://gitbox.apache.org/repos/asf/tomcat.git > > > > > > The following commit(s) were added to refs/heads/master by this push: > > new 4691266 Fix build > > 4691266 is described below > > > > commit 4691266ee48f084e0698ef0233036c0089492248 > > Author: remm <r...@apache.org> > > AuthorDate: Wed May 15 13:56:42 2019 +0200 > > > > Fix build > > > > Maybe it isn't such a good idea to remove throws IOException ... I > can > > add it back depending on the feedback, even if it serves no purpose. > > My current thinking is that I like it. Close the socket and handle > (i.e.log) the potential exception in a single place. It isn't as if the > calling code is going to do anything different depending on whether the > socket closed cleanly or not. > Ok. I'm pretty sure I had removed it in the previous refactoring, but I put it back in before commit to avoid breaking the API. This time I got lead from one change to the other and I did make some changes to doClose, so I left it in. I revisited the close issue again along with the multiple pollers feature, since I had looked recently at the ApacheCon 2017 presentation from Huxing ("The Challenges Tomcat Faces in High Throughput Production Systems"), and I understood that: - Robust close and recycle is good (no putting channel twice in the stack ...) - More than one poller helps mess things up The bugs got fixed some time ago of course (and maybe refixed some more), but ultimately it is hard to verify and I preferred to remove all that stuff since it wasn't not needed. Also it should do item 3 of tomcat-next, except the tracing part. Rémy > > Mark > > > > --- > > java/org/apache/coyote/http2/Http2UpgradeHandler.java | 6 > +----- > > .../apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java | 2 > +- > > 2 files changed, 2 insertions(+), 6 deletions(-) > > > > diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java > b/java/org/apache/coyote/http2/Http2UpgradeHandler.java > > index 134e501..585a0e5 100644 > > --- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java > > +++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java > > @@ -1066,11 +1066,7 @@ class Http2UpgradeHandler extends AbstractStream > implements InternalHttpUpgradeH > > // longer required (also notifies any threads waiting for > allocations). > > stream.receiveReset(Http2Error.CANCEL.getCode()); > > } > > - try { > > - socketWrapper.close(); > > - } catch (IOException ioe) { > > - log.debug(sm.getString("upgradeHandler.socketCloseFailed"), > ioe); > > - } > > + socketWrapper.close(); > > } > > > > > > diff --git > a/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java > b/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java > > index 301d25c..0cd91ef 100644 > > --- > a/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java > > +++ > b/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java > > @@ -220,7 +220,7 @@ public class WsRemoteEndpointImplServer extends > WsRemoteEndpointImplBase { > > } > > try { > > socketWrapper.close(); > > - } catch (IOException e) { > > + } catch (Exception e) { > > if (log.isInfoEnabled()) { > > > > log.info(sm.getString("wsRemoteEndpointServer.closeFailed"), > e); > > } > > > > > > --------------------------------------------------------------------- > > 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 > >
