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 <[email protected]> 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
