Re: RFR: 8035653: InetAddress.getLocalHost crash
Thanks for jumping on this Michael, one of my recent changes caused the problem. Your fix looks good to me. Trivially, the test has a 2006 copyright header. -Chris. On 24/02/14 17:55, Michael McMahon wrote: Could I get the following change reviewed please? http://cr.openjdk.java.net/~michaelm/8035653/webrev.1/ We overlooked one place where JNI native field initialization is required in Windows Vista+ Thanks Michael.
RFR: 8035653: InetAddress.getLocalHost crash
Could I get the following change reviewed please? http://cr.openjdk.java.net/~michaelm/8035653/webrev.1/ We overlooked one place where JNI native field initialization is required in Windows Vista+ Thanks Michael.
Re: RFR: JDK-8015692 - java.net.BindException is thrown on Windows XP when HTTP server is started and stopped in the loop.
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 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
Re: RFR [9]: 8034174 Remove use of JVM_* functions from java.net code
Latest webrev: http://chhegar.ie.oracle.com/chhegar/repos/jdk9/dev/dev/jdk/8034174/webrev.01/webrev/ -Chris. On 24/02/14 14:12, Michael McMahon wrote: On 24/02/14 14:09, Chris Hegarty wrote: On 24/02/14 10:42, Michael McMahon wrote: On 23/02/14 08:55, Chris Hegarty wrote: On 22 Feb 2014, at 17:23, Dmitry Samersoff wrote: Chris, Didn't look to windows part. Unix part looks good for me. See also below. I'm a bit concerned because of mixing NET_* abstractions and direct call to OS functions. It might be better to create NET_socket etc. Me too. It is already a mess. System calls should be used directly, unless there is a reason not to do so. We use NET_GetSockOpt/NET_SetSockOpt in one places and plain os functions in other ones it have to be unified. If there is no reason to call the NET_ variant, then the system call should be used. Seems like the big #ifdef in net_util_md.h on this is more or less redundant now since the #define of NET_xxx to JVM_xxx was its only purpose. The only difference between these now is that the bsd/linux variant are defined in a separate file ( extern ), bsd_close/linux_close. I'm not sure, but I think the use of extern is still required here. I think extern would be okay in the other case though. All C functions are extern unless declared static afaik. I wonder would it also be useful to expand the comment just above those definitions that currently just relates to AIX and say which other operating systems it applies to and if we could identify which system calls it affects, and which mean the NET_xx functions must be used. Or maybe this is going beyond what you wanted to do here? Beyond ;-) There is still a lot of cleanup that I want to make to this code, but I'd like to do it incrementally, starting with breaking the dependency on the VM interface. This makes it easier, certainly from a review point of view. -Chris. Michael
Re: RFR [9]: 8034174 Remove use of JVM_* functions from java.net code
Chris, You probably need to modify jdk/make/mapfiles/libnet/mapfile-vers -Dmitry On 2014-02-24 18:19, Chris Hegarty wrote: > On 24/02/14 14:12, Michael McMahon wrote: >> On 24/02/14 14:09, Chris Hegarty wrote: >>> On 24/02/14 10:42, Michael McMahon wrote: On 23/02/14 08:55, Chris Hegarty wrote: > On 22 Feb 2014, at 17:23, Dmitry Samersoff > wrote: > >> Chris, >> >> Didn't look to windows part. Unix part looks good for me. See also >> below. >> >> I'm a bit concerned because of mixing NET_* abstractions and direct >> call >> to OS functions. It might be better to create NET_socket etc. > Me too. It is already a mess. System calls should be used directly, > unless there is a reason not to do so. > >> We use NET_GetSockOpt/NET_SetSockOpt in one places and plain os >> functions in other ones it have to be unified. > If there is no reason to call the NET_ variant, then the system call > should be used. Seems like the big #ifdef in net_util_md.h on this is more or less redundant now since the #define of NET_xxx to JVM_xxx was its only purpose. >>> >>> The only difference between these now is that the bsd/linux variant >>> are defined in a separate file ( extern ), bsd_close/linux_close. I'm >>> not sure, but I think the use of extern is still required here. >>> >> >> I think extern would be okay in the other case though. All C functions >> are extern unless >> declared static afaik. > > Thanks. I'll include extern, and remove the other definitions. > > -Chris. > >> I wonder would it also be useful to expand the comment just above those definitions that currently just relates to AIX and say which other operating systems it applies to and if we could identify which system calls it affects, and which mean the NET_xx functions must be used. Or maybe this is going beyond what you wanted to do here? >>> >>> Beyond ;-) There is still a lot of cleanup that I want to make to this >>> code, but I'd like to do it incrementally, starting with breaking the >>> dependency on the VM interface. This makes it easier, certainly from a >>> review point of view. >>> >>> -Chris. >>> Michael >> -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
Re: RFR [9]: 8034174 Remove use of JVM_* functions from java.net code
On 24/02/14 14:12, Michael McMahon wrote: On 24/02/14 14:09, Chris Hegarty wrote: On 24/02/14 10:42, Michael McMahon wrote: On 23/02/14 08:55, Chris Hegarty wrote: On 22 Feb 2014, at 17:23, Dmitry Samersoff wrote: Chris, Didn't look to windows part. Unix part looks good for me. See also below. I'm a bit concerned because of mixing NET_* abstractions and direct call to OS functions. It might be better to create NET_socket etc. Me too. It is already a mess. System calls should be used directly, unless there is a reason not to do so. We use NET_GetSockOpt/NET_SetSockOpt in one places and plain os functions in other ones it have to be unified. If there is no reason to call the NET_ variant, then the system call should be used. Seems like the big #ifdef in net_util_md.h on this is more or less redundant now since the #define of NET_xxx to JVM_xxx was its only purpose. The only difference between these now is that the bsd/linux variant are defined in a separate file ( extern ), bsd_close/linux_close. I'm not sure, but I think the use of extern is still required here. I think extern would be okay in the other case though. All C functions are extern unless declared static afaik. Thanks. I'll include extern, and remove the other definitions. -Chris. I wonder would it also be useful to expand the comment just above those definitions that currently just relates to AIX and say which other operating systems it applies to and if we could identify which system calls it affects, and which mean the NET_xx functions must be used. Or maybe this is going beyond what you wanted to do here? Beyond ;-) There is still a lot of cleanup that I want to make to this code, but I'd like to do it incrementally, starting with breaking the dependency on the VM interface. This makes it easier, certainly from a review point of view. -Chris. Michael
Re: RFR [9]: 8034174 Remove use of JVM_* functions from java.net code
On 24/02/14 14:09, Chris Hegarty wrote: On 24/02/14 10:42, Michael McMahon wrote: On 23/02/14 08:55, Chris Hegarty wrote: On 22 Feb 2014, at 17:23, Dmitry Samersoff wrote: Chris, Didn't look to windows part. Unix part looks good for me. See also below. I'm a bit concerned because of mixing NET_* abstractions and direct call to OS functions. It might be better to create NET_socket etc. Me too. It is already a mess. System calls should be used directly, unless there is a reason not to do so. We use NET_GetSockOpt/NET_SetSockOpt in one places and plain os functions in other ones it have to be unified. If there is no reason to call the NET_ variant, then the system call should be used. Seems like the big #ifdef in net_util_md.h on this is more or less redundant now since the #define of NET_xxx to JVM_xxx was its only purpose. The only difference between these now is that the bsd/linux variant are defined in a separate file ( extern ), bsd_close/linux_close. I'm not sure, but I think the use of extern is still required here. I think extern would be okay in the other case though. All C functions are extern unless declared static afaik. I wonder would it also be useful to expand the comment just above those definitions that currently just relates to AIX and say which other operating systems it applies to and if we could identify which system calls it affects, and which mean the NET_xx functions must be used. Or maybe this is going beyond what you wanted to do here? Beyond ;-) There is still a lot of cleanup that I want to make to this code, but I'd like to do it incrementally, starting with breaking the dependency on the VM interface. This makes it easier, certainly from a review point of view. -Chris. Michael
Re: RFR [9]: 8034174 Remove use of JVM_* functions from java.net code
On 24/02/14 10:42, Michael McMahon wrote: On 23/02/14 08:55, Chris Hegarty wrote: On 22 Feb 2014, at 17:23, Dmitry Samersoff wrote: Chris, Didn't look to windows part. Unix part looks good for me. See also below. I'm a bit concerned because of mixing NET_* abstractions and direct call to OS functions. It might be better to create NET_socket etc. Me too. It is already a mess. System calls should be used directly, unless there is a reason not to do so. We use NET_GetSockOpt/NET_SetSockOpt in one places and plain os functions in other ones it have to be unified. If there is no reason to call the NET_ variant, then the system call should be used. Seems like the big #ifdef in net_util_md.h on this is more or less redundant now since the #define of NET_xxx to JVM_xxx was its only purpose. The only difference between these now is that the bsd/linux variant are defined in a separate file ( extern ), bsd_close/linux_close. I'm not sure, but I think the use of extern is still required here. I wonder would it also be useful to expand the comment just above those definitions that currently just relates to AIX and say which other operating systems it applies to and if we could identify which system calls it affects, and which mean the NET_xx functions must be used. Or maybe this is going beyond what you wanted to do here? Beyond ;-) There is still a lot of cleanup that I want to make to this code, but I'd like to do it incrementally, starting with breaking the dependency on the VM interface. This makes it easier, certainly from a review point of view. -Chris. Michael
Re: RFR [9]: 8034174 Remove use of JVM_* functions from java.net code
On 23/02/14 08:55, Chris Hegarty wrote: On 22 Feb 2014, at 17:23, Dmitry Samersoff wrote: Chris, Didn't look to windows part. Unix part looks good for me. See also below. I'm a bit concerned because of mixing NET_* abstractions and direct call to OS functions. It might be better to create NET_socket etc. Me too. It is already a mess. System calls should be used directly, unless there is a reason not to do so. We use NET_GetSockOpt/NET_SetSockOpt in one places and plain os functions in other ones it have to be unified. If there is no reason to call the NET_ variant, then the system call should be used. Seems like the big #ifdef in net_util_md.h on this is more or less redundant now since the #define of NET_xxx to JVM_xxx was its only purpose. I wonder would it also be useful to expand the comment just above those definitions that currently just relates to AIX and say which other operating systems it applies to and if we could identify which system calls it affects, and which mean the NET_xx functions must be used. Or maybe this is going beyond what you wanted to do here? Michael
