On Fri, Jan 12, 2018 at 9:27 AM, Mark Thomas <ma...@apache.org> wrote:
> On 12/01/18 08:04, Rémy Maucherat wrote: > > On Fri, Jan 12, 2018 at 12:06 AM, Mark Thomas <ma...@apache.org> wrote: > > > >> Hi, > >> > >> I've been looking at how we close NIO channels and I think there is an > >> opportunity for a little clean-up that, in turn, may allow a little > >> de-duplication between NIO and NIO2. > >> > >> Currently, in various places in the codebase we close an NIO channel > >> using some variation of: > >> > >> channel.socket().close(); > >> channel.close(); > >> > >> My reading of the Javadoc, source code and some debugging suggests that > >> these lines are equivalent and that we can simply do: > >> > >> channel.close(); > >> > >> Across the codebase, you end up with a patch like this: > >> http://people.apache.org/~markt/patches/2018-01-11- > >> channel-close-tc9-v1.patch > >> > >> Before I apply this patch to trunk, can anyone see anything I am missing > >> here? Is there a reason to keep the code as it is? > >> > > Interesting attempt. As usual, the main reason to keep the code as is is: > > it works :) > > Fair point. Working is definitely a good thing :) > > But, if the first line is unnecessary, then there are some small > benefits to removing it. I've run the unit tests (which give pretty good > coverage of I/O edge cases) and they all pass with the patch applied. > > On the other hand, if the change is viewed as too risky, I'd be happy to > put it on the back-burner until we start thinking about Tomcat 10. > > I have no issue with trying it. Rémy
