RFR: JDK-8015692 - java.net.BindException is thrown on Windows XP when HTTP server is started and stopped in the loop.

[Chris Hegarty]

On 12 Nov 2014, at 22:12, Mark Sheppard <mark.sheppard at 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.sheppard at 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
>>> 
>