On 12 Nov 2014, at 22:12, Mark Sheppard <mark.shepp...@oracle.com> wrote:
> Hi > Please oblige and review the updated patch > http://cr.openjdk.java.net/~msheppar/8015692/webrev.02a/ The source change looks fine to me. Just a comment/question; since you are swallowing the IE I think you should restore the interrupt status on the thread. try { dispatcherThread.join(); } catch (InterruptedException e) { Thread.currentThread().interrupt(); <<<< HERE logger.log(Level.FINER, "ServerImpl.stop: ", e); } The test could be simplified somewhat by creating one HttpServer, getting its port (getServerAddress()) , and then re-using that port. Maybe a little less #’s in the output ? ;-) -Chris. > to address the issue > https://bugs.openjdk.java.net/browse/JDK-8015692 > > as suggested the ServerImpl.stop() method performs a join, only, for the > dispatcher thread > so that all housekeeping is done prior to exiting stop. > > regards > Mark > > On 24/02/2014 17:08, Chris Hegarty wrote: >> Hi Mark, >> >> I think join should be sufficient here. >> >> I understand your argument to move selector close into stop, but that just >> seems to require extra co-ordination between stop and the dispatcher loop, >> namely you now need to check if the selector is closed in a few places. I >> think it is simpler to leave the original code as is, dispatcher closes the >> selector, and only selectNow is invoked from stop. Or maybe I'm missing >> something. >> >> -Chris. >> >> On 21/02/14 12:21, Mark Sheppard wrote: >>> Hi Chris, >>> thanks for the response. >>> >>> Yes, that's true. >>> It was just the way it evolved as I analyzed the issue. >>> Originally, the join was after the close and selectNow. >>> The close was moved from Dispatcher to stop, as there was some >>> "interplay" between the Dispatcher thread >>> and the stop thread, when the Dispatcher was invoking the close. >>> Then added the join() in the stop method, to ensure that the Dispatcher >>> wasn't still executing after the server had been >>> stopped. >>> >>> As the Selector is opened in the ServerImpl constructor and not in the >>> Dispatcher, it seemed >>> from a symmetry view point more logical to invoke the close in the >>> ServerImpl stop >>> >>> The selectNow is just insurance for cleanup purposes. >>> >>> It is possible that the join should be higher up in the stop() flow i.e. >>> immediately after the >>> setting the finish flag? >>> As such, the Dispatcher should be finished with the various >>> HttpConnection collections, before >>> the stop processes them. >>> >>> regards >>> Mark >>> >>> On 21/02/2014 07:22, Chris Hegarty wrote: >>>> Mark, >>>> >>>> I agree with you, there is certainly some additional co-ordination >>>> needed between the thread invoking the stop method and the dispatcher >>>> thread. >>>> >>>> I wonder why you needed to add the selectNow() and the close() after >>>> you have joined the dispatcher thread? Since you are guaranteed that >>>> the dispatcher thread will have exited before join() returns? >>>> >>>> -Chris. >>>> >>>> On 17 Feb 2014, at 01:20, Mark Sheppard <mark.shepp...@oracle.com> wrote: >>>> >>>>> Hi >>>>> >>>>> Please oblige and review the changes in the webrev >>>>> >>>>> http://cr.openjdk.java.net/~msheppar/8015692/jdk9/webrev/ >>>>> >>>>> to address the issue raised in the bug >>>>> >>>>> https://bugs.openjdk.java.net/browse/JDK-8015692 >>>>> >>>>> Summary: >>>>> a series Junit tests which start stop instances of an >>>>> com.sun.net.httpserver.HttpServer failed due to >>>>> java.net.BindException: Address already in use: bind >>>>> >>>>> This was raised against Windows XP, but the sample test to reproduce >>>>> the issue >>>>> was run on Windows 7, and the problem was seen to occur on this OS also. >>>>> The sample was run against jdk7, jdk8 and jdk9: reproducible on each. >>>>> >>>>> On investigation it appears that some additional co-ordination is >>>>> required between the >>>>> HttpServer's (actually SereverImpl) dispatcher thread and the thread >>>>> invoking the stop >>>>> method. This change has amended the stop method to wait for the >>>>> Dispatcher thread to complete, then >>>>> invokes the selector's selectNow, to handled cancelled events, and >>>>> closes the selector. >>>>> The selector.close() has been removed from the Dispatcher's run method. >>>>> >>>>> regards >>>>> Mark >>> >
