Re: RFR 8169115, java/net/InetAddress/ptr/lookup.sh failed intermittently
Felix, Changes looks good to me. Thank you for doing it. -Dmitry On 2016-12-08 13:35, Felix Yang wrote: > Hi Dmitry, > >I tested your suggested "icann.org" and it indeed works well. Please > review the updated webrev: > > http://cr.openjdk.java.net/~xiaofeya/8169115/webrev.02/ > > Thanks, > Felix > On 2016/12/6 19:16, Dmitry Samersoff wrote: >> Felix, >> >> 1. I'm not sure that javaweb.sfbay.sun.com is the best domain name for >> this test. Could we use different one (e.g. icann.org) >> >> 2. This test run JTREG -> Test VM -> Another VM. Could we optimize >> process usage? >> >> It might be better to create a jtreg "same vm" process that >> >> 1. launch another process with -Djava.net.preferIPv4Stack=true >> that do A and PRT lookup in one run. >> >> 2. Read results of process above, do PTR lookup with default >> settings and compare results. >> >> -Dmitry >> >> >> On 2016-12-06 12:06, Felix Yang wrote: >>> Hi, >>> >>> please review the following patch. It generally coverts codes from >>> shell to plain java. >>> >>> Bug: >>> >>> https://bugs.openjdk.java.net/browse/JDK-8169115 >>> >>> Webrev: >>> >>> http://cr.openjdk.java.net/~xiaofeya/8169115/webrev.00/ >>> >>> Thanks, >>> >>> Felix >>> >> > -- 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 8169115, java/net/InetAddress/ptr/lookup.sh failed intermittently
Felix, 1. I'm not sure that javaweb.sfbay.sun.com is the best domain name for this test. Could we use different one (e.g. icann.org) 2. This test run JTREG -> Test VM -> Another VM. Could we optimize process usage? It might be better to create a jtreg "same vm" process that 1. launch another process with -Djava.net.preferIPv4Stack=true that do A and PRT lookup in one run. 2. Read results of process above, do PTR lookup with default settings and compare results. -Dmitry On 2016-12-06 12:06, Felix Yang wrote: > Hi, > >please review the following patch. It generally coverts codes from > shell to plain java. > > Bug: > > https://bugs.openjdk.java.net/browse/JDK-8169115 > > Webrev: > > http://cr.openjdk.java.net/~xiaofeya/8169115/webrev.00/ > > Thanks, > > Felix > -- 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 8075484:SocketInputStream.socketRead0 can hang even with soTimeout set
Vyom,
Looks good for me!
SocketInputStream.c:68
It's better to check for both EAGAIN and EINTR after poll()
(no need to re-review).
-Dmitry
On 2016-09-06 12:20, Vyom Tewari wrote:
> Hi,
>
> Please find the latest
> webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.2/index.html
> <http://cr.openjdk.java.net/%7Evtewari/8075484/webrev0.2/index.html>). I
> have incorporated the review comments.
>
> Thanks,
>
> Vyom
>
>
> On Monday 05 September 2016 08:37 PM, Chris Hegarty wrote:
>> On 05/09/16 15:37, Mark Sheppard wrote:
>>>
>>> if the desire is to avoid making double calls on gettimeofday in the
>>> NET_ReadWithTimeout's while loop for its main call flow,
>>
>> Yes, the desire is to make no more calls to gettimeofday, than are
>> already done.
>>
>>> then consider a modification to NET_Timeout and create a version
>>> int NET_TimeoutWithCurrentTime(int s, long timeout, stuct timeval *
>>> current_time)
>>>
>>> int NET_TimeoutWithCurrentTime(int s, long timeout, stuct timeval *
>>> current_time) {
>>> int result;
>>> struct timeval t;
>>> long prevtime, newtime;
>>> struct pollfd pfd;
>>> pfd.fd = s;
>>> pfd.events = POLLIN;
>>>
>>> if (timeout > 0) {
>>> prevtime = (current_time->tv_sec * 1000) +
>>> current_time->tv_usec / 1000;
>>> }
>>>
>>> for(;;) {
>>> result = poll(, 1, timeout);
>>> if (result < 0 && errno == EINTR) {
>>> if (timeout > 0) {
>>> gettimeofday(, NULL);
>>> newtime = (t.tv_sec * 1000) + t.tv_usec /1000;
>>> timeout -= newtime - prevtime;
>>> if (timeout <= 0)
>>> return 0;
>>> prevtime = newtime;
>>> }
>>> } else {
>>> return result;
>>> }
>>> }
>>> }
>>>
>>> the NET_ReadWithTimeout is modified with.
>>>
>>>while (timeout > 0) {
>>> result = NET_TimeoutWithCurrentTime(fd, timeout, );
>>>
>>> in any case there is the potential for multiple invocation of
>>> gettimeofday due to a need to make
>>> poll restartable,
>>
>> Yes, and that is fine. Just no more than is already there.
>>
>> and adjust the originally specified timeout
>>> accordingly, and for the edge case
>>> after the non blocking read.
>>
>> After an initial skim, your suggestion seems reasonable.
>>
>> -Chris.
>>
>>> regards
>>> Mark
>>>
>>>
>>>
>>> On 05/09/2016 12:02, Chris Hegarty wrote:
>>>> Vyom,
>>>>
>>>> >>>
>>>> webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.1/index.html
>>>>
>>>> There is some concern about the potential performance effect, etc,
>>>> of gettimeofday, maybe there is a way out of this. The reuse of
>>>> NET_Timeout is good, but it also calls gettimeofday. It seems that
>>>> a specific NET_ReadWithTimeout could be written to NOT reuse
>>>> NET_Timeout, but effectively inline its interruptible operation.
>>>> Or write a variant of NET_Timeout that takes a function to
>>>> execute. Rather than effectively two loops conditioned on the
>>>> timeout. Disclaimer: I have not actually tried to do this, but
>>>> it seems worth considering / evaluating.
>>>>
>>>> -Chris.
>>>>
>>>> On 02/09/16 04:39, Vyom Tewari wrote:
>>>>> hi Dimitry,
>>>>>
>>>>> thanks for review, I did consider to use a monotonically increasing
>>>>> clock like "clock_gettime" but existing nearby code("NET_Timeout")
>>>>> uses
>>>>> "gettimeofday" so i choose to be consistent with the existing code.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Vyom
>>>>>
>>>>>
>>>>> On Friday 02 September 2016 01:38 AM, Dmitry Samersoff wrote:
>>>>>> Vyom,
>>>>>>
>>>>>> Did you consider to use select() to calculate timeout instead of
>>>>>> gettimeofday ?
>>>>>>
>>>>>> gettimeofday is affected by system
Re: RFR 8075484:SocketInputStream.socketRead0 can hang even with soTimeout set
Vyom,
> thanks for review, I did consider to use a monotonically increasing
> clock like "clock_gettime" but existing nearby code("NET_Timeout")
> uses "gettimeofday" so i choose to be consistent with the existing
> code.
OK. The fix looks good for me.
-Dmitry
On 2016-09-02 06:39, Vyom Tewari wrote:
> hi Dimitry,
>
> thanks for review, I did consider to use a monotonically increasing
> clock like "clock_gettime" but existing nearby code("NET_Timeout") uses
> "gettimeofday" so i choose to be consistent with the existing code.
>
> Thanks,
>
> Vyom
>
>
> On Friday 02 September 2016 01:38 AM, Dmitry Samersoff wrote:
>> Vyom,
>>
>> Did you consider to use select() to calculate timeout instead of
>> gettimeofday ?
>>
>> gettimeofday is affected by system time changes, so running ntpd can
>> cause unpredictable behavior of this code. Also it's rather expensive
>> syscall.
>>
>> -Dmitry
>>
>> On 2016-09-01 19:03, Vyom Tewari wrote:
>>> please find the updated
>>> webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.1/index.html
>>> <http://cr.openjdk.java.net/%7Evtewari/8075484/webrev0.1/index.html>). I
>>> incorporated the review comments.
>>>
>>> Thanks,
>>>
>>> Vyom
>>>
>>>
>>> On Tuesday 30 August 2016 04:11 PM, Mark Sheppard wrote:
>>>> Hi
>>>>perhaps there is an opportunity to do some refactoring here (...
>>>> for me a "goto " carries a code smell! )
>>>>
>>>> along the lines
>>>>
>>>> if (timeout) {
>>>> nread = NET_ReadWithTimeout(...);
>>>> } else {
>>>> nread = NET_Read(...);
>>>> }
>>>>
>>>>
>>>> the NET_ReadWithTimeout (...) function will contain a restructuring of
>>>> your goto loop
>>>> while (_timeout > 0) { nread = NET_Timeout(fd, _timeout);
>>>>if (nread <= 0) {
>>>>if (nread == 0) {
>>>>JNU_ThrowByName(env, JNU_JAVANETPKG
>>>> "SocketTimeoutException",
>>>>"Read timed out");
>>>>} else if (nread == -1) {
>>>>if (errno == EBADF) {
>>>> JNU_ThrowByName(env, JNU_JAVANETPKG
>>>> "SocketException", "Socket closed");
>>>>} else if (errno == ENOMEM) {
>>>>JNU_ThrowOutOfMemoryError(env, "NET_Timeout
>>>> native heap allocation failed");
>>>>} else {
>>>>JNU_ThrowByNameWithMessageAndLastError
>>>>(env, JNU_JAVANETPKG "SocketException",
>>>> "select/poll failed");
>>>>}
>>>>}
>>>> // release buffer in main call flow
>>>> // if (bufP != BUF) {
>>>> // free(bufP);
>>>> // }
>>>> nread = -1;
>>>> break;
>>>>} else {
>>>> nread = NET_NonBlockingRead(fd, bufP, len);
>>>> if (nread == -1 && ((errno == EAGAIN) || (errno == EWOULDBLOCK))) {
>>>>gettimeofday(, NULL);
>>>> newtime = t.tv_sec * 1000 + t.tv_usec / 1000;
>>>> _timeout -= newtime - prevtime;
>>>> if(_timeout > 0){
>>>> prevtime = newtime;
>>>>}
>>>> } else { break; } } } return nread;
>>>>
>>>> e
>>>>
>>>> regards
>>>> Mark
>>>>
>>>>
>>>> On 29/08/2016 10:58, Vyom Tewari wrote:
>>>>> gentle reminder, please review the below code change.
>>>>>
>>>>> Vyom
>>>>>
>>>>>
>>>>> On Monday 22 August 2016 05:12 PM, Vyom Tewari wrote:
>>>>>> Hi All,
>>>>>>
>>>>>> Please review the code changes for below issue.
>>>>>>
>>>>>> Bug : https://bugs.openjdk.java.net/browse/JDK-8075484
>>>>>>
>>>>>> webrev:
>>>>>> http://cr.openjdk.java.net/~vtewari/8075484/webrev0.0/index.html
>>>>>> <http://cr.openjdk.java.net/%7Evtewari/8075484/webrev0.0/index.html>
>>>>>>
>>>>>> This issue is SocketInputStream.socketread0() hangs even with
>>>>>> "soTimeout" set.the implementation of
>>>>>> Java_java_net_SocketInputStream_socketRead0 assumes that read()
>>>>>> won't block after poll() reports that a read is possible.
>>>>>>
>>>>>> This assumption does not hold, as noted on the man page for select
>>>>>> (referenced by the man page for poll): Under Linux, select() may
>>>>>> report a socket file descriptor as "ready for reading", while
>>>>>> nevertheless a subsequent read blocks. This could for example happen
>>>>>> when data has arrived but upon examination has wrong checksum and is
>>>>>> discarded.
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Vyom
>>>>>>
>>>>>>
>>
>
--
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 8075484:SocketInputStream.socketRead0 can hang even with soTimeout set
Vyom,
Did you consider to use select() to calculate timeout instead of
gettimeofday ?
gettimeofday is affected by system time changes, so running ntpd can
cause unpredictable behavior of this code. Also it's rather expensive
syscall.
-Dmitry
On 2016-09-01 19:03, Vyom Tewari wrote:
> please find the updated
> webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.1/index.html
> <http://cr.openjdk.java.net/%7Evtewari/8075484/webrev0.1/index.html>). I
> incorporated the review comments.
>
> Thanks,
>
> Vyom
>
>
> On Tuesday 30 August 2016 04:11 PM, Mark Sheppard wrote:
>>
>> Hi
>> perhaps there is an opportunity to do some refactoring here (...
>> for me a "goto " carries a code smell! )
>>
>> along the lines
>>
>> if (timeout) {
>> nread = NET_ReadWithTimeout(...);
>> } else {
>> nread = NET_Read(...);
>> }
>>
>>
>> the NET_ReadWithTimeout (...) function will contain a restructuring of
>> your goto loop
>> while (_timeout > 0) { nread = NET_Timeout(fd, _timeout);
>> if (nread <= 0) {
>> if (nread == 0) {
>> JNU_ThrowByName(env, JNU_JAVANETPKG
>> "SocketTimeoutException",
>> "Read timed out");
>> } else if (nread == -1) {
>> if (errno == EBADF) {
>>JNU_ThrowByName(env, JNU_JAVANETPKG
>> "SocketException", "Socket closed");
>> } else if (errno == ENOMEM) {
>> JNU_ThrowOutOfMemoryError(env, "NET_Timeout native
>> heap allocation failed");
>> } else {
>> JNU_ThrowByNameWithMessageAndLastError
>> (env, JNU_JAVANETPKG "SocketException",
>> "select/poll failed");
>> }
>> }
>> // release buffer in main call flow
>> // if (bufP != BUF) {
>> // free(bufP);
>> // }
>> nread = -1;
>> break;
>> } else {
>> nread = NET_NonBlockingRead(fd, bufP, len);
>> if (nread == -1 && ((errno == EAGAIN) || (errno == EWOULDBLOCK))) {
>> gettimeofday(, NULL);
>> newtime = t.tv_sec * 1000 + t.tv_usec / 1000;
>> _timeout -= newtime - prevtime;
>> if(_timeout > 0){
>> prevtime = newtime;
>> }
>> } else { break; } } } return nread;
>>
>> e
>>
>> regards
>> Mark
>>
>>
>> On 29/08/2016 10:58, Vyom Tewari wrote:
>>> gentle reminder, please review the below code change.
>>>
>>> Vyom
>>>
>>>
>>> On Monday 22 August 2016 05:12 PM, Vyom Tewari wrote:
>>>> Hi All,
>>>>
>>>> Please review the code changes for below issue.
>>>>
>>>> Bug : https://bugs.openjdk.java.net/browse/JDK-8075484
>>>>
>>>> webrev:
>>>> http://cr.openjdk.java.net/~vtewari/8075484/webrev0.0/index.html
>>>> <http://cr.openjdk.java.net/%7Evtewari/8075484/webrev0.0/index.html>
>>>>
>>>> This issue is SocketInputStream.socketread0() hangs even with
>>>> "soTimeout" set.the implementation of
>>>> Java_java_net_SocketInputStream_socketRead0 assumes that read()
>>>> won't block after poll() reports that a read is possible.
>>>>
>>>> This assumption does not hold, as noted on the man page for select
>>>> (referenced by the man page for poll): Under Linux, select() may
>>>> report a socket file descriptor as "ready for reading", while
>>>> nevertheless a subsequent read blocks. This could for example happen
>>>> when data has arrived but upon examination has wrong checksum and is
>>>> discarded.
>>>>
>>>> Thanks,
>>>>
>>>> Vyom
>>>>
>>>>
>>>
>>
>
--
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: Help needed: Java Socket and close detection
Christoph, My $0.2 Typically you see RST packet when the data come to a *closed* socket. You shouldn't get RST if client/server communication shutdown properly. Also balancer may take a care about connection shutdown (it needs to update internal tables) so it's possible that you never get RST after balancer. Moreover, there are RST based attack and scanning methods so firewall might be set to reject RST packets and doesn't pass it to inside network. I.e. IMHO, customer server should be updated to live without RST. -Dmitry On 2016-07-13 18:29, Langer, Christoph wrote: > Hi folks, > > > > I have a question to the experts - regarding an issue that was reported > to me by a customer. > > > > In the customer scenario they are running a Servlet engine and the > Servlet is constantly sending data to a browser client. When the browser > client is closed, the server does not get a notification of the other > end having been terminated and is constantly sending out data and > blocking an application thread. I’m under the assumption that the server > should get an RST packet from the network upon writing/flushing data to > the OutputStream as soon as the client is gone and hence an Exception > should pop up but this isn’t happening. > > > > There is a load balancer and maybe other network infrastructure involved > in between the Servlet JVM and the browser client. We did some TCPDUMP > tracing at the load balancer and we could not see an RST packet coming > in from the client side. But when I’m running the scenario without all > this network infrastructure involved, e.g. between servers and clients > in the same network, I would always observe an RST packet once I close > the browser. A FIN packet is received, too, but this does not lead to an > Exception and to all my knowledge this can’t be detected, not from the > java Socket API and even less from the Servlet API which is just dealing > with Streams. > > > > So my question to the experts is most of all: Would you agree that an > RST packet should be generated in the network and received by the > server? Or is it a normal behavior that servers must deal with not > receiving RSTs and hence needing to wait for a timeout until e.g. the > load balancer generates an RST? Also, is there any way to detect a FIN > in the JVM and react on it? > > > > Thanks in advance and best regards > > Christoph > > > -- 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: 8158519: Incorrect network mask and broadcast address on Linux when there are multiple IPv4 addresses on an interface
Chris,
Looks good for me. Only minor nits (no need to re-review).
1040 missed space before {
1158 please, add comment and {} around continue.
1938 I'm not sure that check for IFF_POINTOPOINT is necessary (but
don't mid to leave it). Do you know the case when both flags are set?
-Dmitry
On 2016-06-02 15:06, Chris Hegarty wrote:
> Doychin, et al,
>
> I finally got time to look at the issue you reported and your suggested
> patch. I filed
> 8158519 [1] to track the issue. I think your suggested patch may be ok, but I
> just
> wanted to push on the ioctl approach. I found an alternative, and verified
> that it
> works as expected. Please take a look, and verify in your environment. Then
> we
> need to weigh up the two separate approaches.
>
> http://cr.openjdk.java.net/~chegar/8158519.00/
>
> For the record, I don’t have any specific issue with using getifaddrs, I just
> wanted to
> see if there was a less invasive change. That said, using getifaddrs could
> lead to
> cleaner code, but we would need to check the situation on AIX ( which I don’t
> have
> access to ).
>
> -Chris.
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8158519
>
Re: RFR 8157811 [9] Additional minor fixes and cleanups in Networking native code
Chris,
Looks good for me!
On 2016-05-25 16:22, Chris Hegarty wrote:
> Thanks. I included it, though it may not show in the webrev ( white space
> change )
>
> I generalise the issue to cover a few other minor issues.
>
> Webrev updated in-place:
> http://cr.openjdk.java.net/~chegar/8157811/
>
> -Chris.
>
>> On 25 May 2016, at 13:59, Langer, Christoph <christoph.lan...@sap.com> wrote:
>>
>> Hi Chris,
>>
>> looks nice, I had seen some of these places, too.
>>
>> Here is another one which you could add:
>>
>> --- a/src/java.base/unix/native/libnet/NetworkInterface.c Tue May 24
>> 10:14:41 2016 -0700
>> +++ b/src/java.base/unix/native/libnet/NetworkInterface.c Wed May 25
>> 14:56:31 2016 +0200
>> @@ -1829,7 +1829,7 @@
>> strncpy(if2.lifr_name, ifname, sizeof(if2.lifr_name) - 1);
>>
>> if (ioctl(sock, SIOCGLIFFLAGS, (char *)) < 0) {
>> - return -1;
>> +return -1;
>> }
>>
>> *flags = if2.lifr_flags;
>>
>> Best regards
>> Christoph
>>
>>> -Original Message-
>>> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Chris
>>> Hegarty
>>> Sent: Mittwoch, 25. Mai 2016 11:50
>>> To: OpenJDK Network Dev list <net-dev@openjdk.java.net>
>>> Subject: RFR 8157811 [9] Additional minor fixes and cleanups in
>>> NetworkInterface.c
>>>
>>> As a follow up to JDK-8156521, and when comparing against the
>>> version of this file in jdk8u-dev a few minor issues were noticed.
>>>
>>> There is a free of a memory structure that was missed somehow
>>> in the 9 version of this file ( it is already in the 8 version ). The
>>> remaining few changes are just some minor cleanup.
>>>
>>> http://cr.openjdk.java.net/~chegar/8157811/
>>> https://bugs.openjdk.java.net/browse/JDK-8157811
>>>
>>> -Chris.
>
--
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(S): 8156521: Minor Fixes and cleanups in NetworkInterface.c
Christoph, Looks good for me (Reviewed). Few more space nits (no need to re-review) 1141 Missed space around = in i=0 1890 extra space after ( 2068 wrong indent -Dmitry On 2016-05-10 12:54, Langer, Christoph wrote: > Hi all, > > > > can I please get a review for a change to NetworkInterface.c > > > > bugreport: https://bugs.openjdk.java.net/browse/JDK-8156521 > > webrev: http://cr.openjdk.java.net/~clanger/webrevs/8156521.0/ > > > > Apart from quite a few whitespace changes to clean up the coding, I went > through and replaced all occurences of strcpy with strncpy as this was a > finding of a code scanner that we used. Also in function > “enumIPv6Interfaces” for Linux the local variable plen was changed from > int to short. > > > > I ran builds on Linux, AIX, Solaris Sparc and Darwin to make sure > nothing broke. > > > > Thanks in advance > > Christoph > > > -- 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: DNS resolution fails after resolv.conf update
Stanislav, I don't think we should fix it on JDK level. Workaround is obvious and actually recommended for production - install one of cashing nameservers and lock resolv.conf to localhost. -Dmitry On 2015-05-04 18:38, Stanislav Baiduzhyi wrote: Hi All, We are facing an issue with DNS server caching on RHEL-based distros: after the update of resolv.conf java application cannot resolve the hosts any more. Reproducer is very simple: 1. Clean /etc/resolv.conf or connect to vpn and use vpn-only nameserver. 2. Launch the minimal java app [1]. 3. Restore the /etc/resolv.conf or disconnect from vpn (/etc/resolv.conf should be updated with accessible nameserver at this moment). 4. Notice that name resolution continues to fail. I've prepared a small webrev that fixes the issue [2], but would like to hear some ideas/criticism first. If the patch looks ok to everyone, please create an issue for this, and I will submit it for approval/sponsorship. [1]: https://e61b615da46327c9050d624b0a3783ba21c6b125.googledrive.com/host/0B5Kp-cB1sXJrfnZ4NHZ0S1V3UTJZcDFra3RwUFZjQXY5WVZzUkwtTTd0Z1IyR1JmbDZPSVk/resolvconf-reinit/ResolvingHost.java [2]: https://e61b615da46327c9050d624b0a3783ba21c6b125.googledrive.com/host/0B5Kp-cB1sXJrfnZ4NHZ0S1V3UTJZcDFra3RwUFZjQXY5WVZzUkwtTTd0Z1IyR1JmbDZPSVk/resolvconf-reinit/ -- 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 8067105: Socket returned by ServerSocket.accept() is inherited by child process on Windows
Chris, In a windows world it's rather common to rely on socket inheritance e.g. set socket to stdin(stdout) of child process and do CreateProcess. So please make sure your changes don't have side effects, also it might be better to control handler inheritance in one place only (at CreateProcess call) and don't manage it explicitly in other places. -Dmitry On 2014-12-17 18:47, Chris Hegarty wrote: A socket connection which is returned by ServerSocket.accept() is inherited by a child process. The expected behavior is that the socket connection is not inherited by the child process. This is an oversight in the original implementation, that only sets HANDLE_FLAG_INHERIT for newly created sockets. The native socket returned by ServerSocket.accept() should be configured so it will not be inherited by a child process, SetHandleInformation(HANDLE, HANDLE_FLAG_INHERIT, FALSE) . The change is in Java_java_net_DualStackPlainSocketImpl_accept0 diff --git a/src/java.base/windows/native/libnet/DualStackPlainSocketImpl.c b/src/java.base/windows/native/libnet/DualStackPlainSocketImpl.c --- a/src/java.base/windows/native/libnet/DualStackPlainSocketImpl.c +++ b/src/java.base/windows/native/libnet/DualStackPlainSocketImpl.c @@ -294,6 +294,8 @@ return -1; } +SetHandleInformation((HANDLE)(UINT_PTR)newfd, HANDLE_FLAG_INHERIT, FALSE); + ia = NET_SockaddrToInetAddress(env, (struct sockaddr *)sa, port); isa = (*env)-NewObject(env, isa_class, isa_ctorID, ia, port); (*env)-SetObjectArrayElement(env, isaa, 0, isa); -Chris. -- 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 8067105: Socket returned by ServerSocket.accept() is inherited by child process on Windows
Chris, OK, but there can be no expectation that something like this can work with java.net.Socket, or SocketChannel. I am only proposing to change these two specific implementations. OK. Thank you for clarification. The fix looks good for me. -Dmitry On 2014-12-18 15:52, Chris Hegarty wrote: On 18 Dec 2014, at 12:22, Dmitry Samersoff dmitry.samers...@oracle.com wrote: Chris, In a windows world it's rather common to rely on socket inheritance e.g. set socket to stdin(stdout) of child process and do CreateProcess. OK, but there can be no expectation that something like this can work with java.net.Socket, or SocketChannel. I am only proposing to change these two specific implementations. So please make sure your changes don't have side effects, The changes will have an effect, accepted sockets/channels will no longer be inherited. No other potential side-effect is possible. also it might be better to control handler inheritance in one place only (at CreateProcess call) and don't manage it explicitly in other places. There is some handling of a small set of “special” handles in the native process implementation. I don’t think we want to prevent all handles from being inherited. Are you suggesting something different ? -Chris. -Dmitry On 2014-12-17 18:47, Chris Hegarty wrote: A socket connection which is returned by ServerSocket.accept() is inherited by a child process. The expected behavior is that the socket connection is not inherited by the child process. This is an oversight in the original implementation, that only sets HANDLE_FLAG_INHERIT for newly created sockets. The native socket returned by ServerSocket.accept() should be configured so it will not be inherited by a child process, SetHandleInformation(HANDLE, HANDLE_FLAG_INHERIT, FALSE) . The change is in Java_java_net_DualStackPlainSocketImpl_accept0 diff --git a/src/java.base/windows/native/libnet/DualStackPlainSocketImpl.c b/src/java.base/windows/native/libnet/DualStackPlainSocketImpl.c --- a/src/java.base/windows/native/libnet/DualStackPlainSocketImpl.c +++ b/src/java.base/windows/native/libnet/DualStackPlainSocketImpl.c @@ -294,6 +294,8 @@ return -1; } +SetHandleInformation((HANDLE)(UINT_PTR)newfd, HANDLE_FLAG_INHERIT, FALSE); + ia = NET_SockaddrToInetAddress(env, (struct sockaddr *)sa, port); isa = (*env)-NewObject(env, isa_class, isa_ctorID, ia, port); (*env)-SetObjectArrayElement(env, isaa, 0, isa); -Chris. -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources. -- 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: JDK-8058932 - java/net/InetAddress/IPv4Formats.java failed because hello.foo.bar does exist
Looks good for me!
-Dmitry
On 2014-10-01 02:26, Mark Sheppard wrote:
Thanks Tom and Dmitry
last up best dressed ...
.invalid as the test domain is a good recommendation
change is now
--- a/test/java/net/InetAddress/IPv4Formats.javaTue Sep 30
13:25:04 2014 +0100
+++ b/test/java/net/InetAddress/IPv4Formats.javaTue Sep 30
23:23:46 2014 +0100
@@ -36,7 +36,7 @@
{126.1, 126.0.0.1},
{128.50.65534, 128.50.255.254},
{192.168.1.2, 192.168.1.2},
-{hello.foo.bar, null},
+{invalidhost.invalid, null},
{1024.1.2.3, null},
{128.14.66000, null }
regards
Mark
On 30/09/2014 18:53, Dmitry Samersoff wrote:
Mark,
It probably should be some-name.invalid
IANA reserve .invalid TLD for tests like this one
see:
http://www.iana.org/assignments/special-use-domain-names/special-use-domain-names.xhtml
-Dmitry
On 2014-09-30 19:21, Mark Sheppard wrote:
Hi
Please oblige and review the following small change to test
test/java/net/InetAddress/IPv4Formats.java
--- a/test/java/net/InetAddress/IPv4Formats.javaTue Sep 30
13:25:04 2014 +0100
+++ b/test/java/net/InetAddress/IPv4Formats.javaTue Sep 30
15:11:05 2014 +0100
@@ -36,7 +36,7 @@
{126.1, 126.0.0.1},
{128.50.65534, 128.50.255.254},
{192.168.1.2, 192.168.1.2},
-{hello.foo.bar, null},
+{somehost.some-domain, null},
{1024.1.2.3, null},
{128.14.66000, null }
which addresses the issue
https://bugs.openjdk.java.net/browse/JDK-8058932
ping hello.foo.bar
Pinging hello.foo.bar [127.0.53.53] with 32 bytes of data:
Reply from 127.0.53.53: bytes=32 time1ms TTL=128
Reply from 127.0.53.53: bytes=32 time1ms TTL=128
Reply from 127.0.53.53: bytes=32 time1ms TTL=128
Reply from 127.0.53.53: bytes=32 time1ms TTL=128
this highlights a DNS configuration issue as indicated in
https://www.icann.org/resources/pages/name-collision-2013-12-06-en
so we remove foo.bar from the test and replace with somehost.some-domain
regards
Mark
--
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: JDK-8058932 - java/net/InetAddress/IPv4Formats.java failed because hello.foo.bar does exist
Mark,
It probably should be some-name.invalid
IANA reserve .invalid TLD for tests like this one
see:
http://www.iana.org/assignments/special-use-domain-names/special-use-domain-names.xhtml
-Dmitry
On 2014-09-30 19:21, Mark Sheppard wrote:
Hi
Please oblige and review the following small change to test
test/java/net/InetAddress/IPv4Formats.java
--- a/test/java/net/InetAddress/IPv4Formats.javaTue Sep 30
13:25:04 2014 +0100
+++ b/test/java/net/InetAddress/IPv4Formats.javaTue Sep 30
15:11:05 2014 +0100
@@ -36,7 +36,7 @@
{126.1, 126.0.0.1},
{128.50.65534, 128.50.255.254},
{192.168.1.2, 192.168.1.2},
-{hello.foo.bar, null},
+{somehost.some-domain, null},
{1024.1.2.3, null},
{128.14.66000, null }
which addresses the issue
https://bugs.openjdk.java.net/browse/JDK-8058932
ping hello.foo.bar
Pinging hello.foo.bar [127.0.53.53] with 32 bytes of data:
Reply from 127.0.53.53: bytes=32 time1ms TTL=128
Reply from 127.0.53.53: bytes=32 time1ms TTL=128
Reply from 127.0.53.53: bytes=32 time1ms TTL=128
Reply from 127.0.53.53: bytes=32 time1ms TTL=128
this highlights a DNS configuration issue as indicated in
https://www.icann.org/resources/pages/name-collision-2013-12-06-en
so we remove foo.bar from the test and replace with somehost.some-domain
regards
Mark
--
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
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 dmitry.samers...@oracle.com 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
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. We use NET_GetSockOpt/NET_SetSockOpt in one places and plain os functions in other ones it have to be unified. (not to your changes, but as far as you touched it) Doing socklen_t n = sizeof(m) it's better cast to socklen_t explicitly - on some platform socklen_t expands to int but sizeof to unsigned so it can cause a compiler warning. It's better to unify check of return value of os functions either as == -1 or as 0 1. net_util.c Do we really need to check JNI_VERSION ? 2. Inet4AddressImpl.c 73, 335 it's better to use NI_MAXHOST in both places 784 optlen should be socklen_t 3. Inet6AddressImpl.c 73, 143 it's better to use NI_MAXHOST in both places 4. net_util_md.c 235 gettimeofday is obsoleted and might be not available on all platforms. So it's better to try clock_gettime first -Dmitry On 2014-02-22 12:29, Chris Hegarty wrote: Interruptible I/O on Solaris has been highly problematic and the long standing plan has been to remove it from the JDK. In JDK6 the VM option UseVMInterruptibleIO was introduced to allow developers/customers experiment with disabling it. In JDK7 the default value of UseVMInterruptibleIO was changed to be false so that it is disabled by default. It is now finally being removed. This bug tracks changing the native in src/share/native/java/net and src/solaris/native/java/net so that the system calls are used directly rather than going through the JVM_* functions. http://cr.openjdk.java.net/~chegar/8034174/webrev.00/webrev/ -Chris. -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
hg: jdk8/tl/jdk: 8024071: In ManagementAgent.start it should be possible to set the jdp.name parameter.
Changeset: 392acefef659 Author:dsamersoff Date: 2013-10-19 20:59 +0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/392acefef659 8024071: In ManagementAgent.start it should be possible to set the jdp.name parameter. Summary: Pass one more property from Agent to JdpController Reviewed-by: jbachorik, sla ! src/share/classes/sun/management/Agent.java ! test/sun/management/jdp/JdpTest.sh ! test/sun/management/jmxremote/startstop/JMXStartStopTest.sh
Re: JDK 8 RFR 8026806: Incomplete test of getaddrinfo() return value could lead to incorrect exception for Windows Inet 6
Brian, Looks good for me (not a reviewer). -Dmitry On 2013-10-17 20:46, Brian Burkhalter wrote: Continuing the discussion from http://mail.openjdk.java.net/pipermail/net-dev/2013-October/007574.html under new issue ID: Issue:https://bugs.openjdk.java.net/browse/JDK-8026806 Webrev: http://cr.openjdk.java.net/~bpb/8026806/webrev/ Thanks, Brian On Oct 17, 2013, at 8:06 AM, Brian Burkhalter wrote: On Oct 17, 2013, at 5:37 AM, Alan Bateman wrote: On 17/10/2013 00:21, Brian Burkhalter wrote: Dmitry, I think you are correct: that slipped by both me and the reviewers. I have reopened the issue and posted an amendment to the original webrev here: http://cr.openjdk.java.net/~bpb/8010371/webrev.4-amendment/ I've restored the bug fields and I assume you'll create a new bug for the follow-up. Sorry this was missed in the original review (probably because it went through so many iterations). -Alan. Yes I will create a new one, thanks. Brian -- 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: JDK 8 RFR 8010371: getaddrinfo can fail with EAI_SYSTEM/EAGAIN, causes UnknownHostException to be thrown
Brian, You have to check error != 0 before call to WSAGetLastError() at ll. 134 Inet6AddressImpl.c Besides that - the fix looks good for me. -Dmitry On 2013-10-14 23:43, Brian Burkhalter wrote: On Oct 14, 2013, at 1:58 AM, Alan Bateman wrote: 2) In Inet4AddressImpl.c and Inet6AddressImpl.c replace NET_ThrowUnknownHostExceptionWithGaiError with NET_ThrowByNameWithLastError (see net_md_util.c). […] If the con of option 2 is acceptable then I think that would be the best way to go, otherwise option 1. Option #2 seems reasonable, the exception messages for similar network conditions are rarely the same on Windows and Unix anyway. Here's the patch updated for this option: http://cr.openjdk.java.net/~bpb/8010371/webrev.4/ However I think it's important to have verified it with one or two errors to be confident that the errors translate as expected. I can do this if we are actually going with this change for JDK 8. One other thing to add is that winsock_errors dates from early versions of Windows whether there wasn't a means to translate Windows Sockets errors. We should look at eliminating it (not for JDK 8 of course, it's too late) so that all errors are handle translated consistently. See https://bugs.openjdk.java.net/browse/JDK-4842142. Brian -- 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: 7180557 - InetAddress.getLocalHost throws UnknownHostException on java7u5 on OSX
Rob,
This version of your fix looks good for me.
Inet4AddressImpl.c:
Thumbs up.
Inet6AddressImpl.c:
Thumbs up.
173 if (JVM_GetHostName(myhostname, NI_MAXHOST)) {
Nitpicking, explicit == -1 would be better here.
Actually, can you tell me why you'd rather not
include ipv6 loopbacks at all?
If interface don't have ipv6 address but we have ipv6 loopback it most
likely means that ipv6 is not functioning properly.
-Dmitry
On 2013-10-04 19:03, Rob McKenna wrote:
Hi Dmitry,
Thanks a lot for the comprehensive review. W.r.t. line 210, I agree
there is a problem with the logic, but I'd like to suggest an
alternative solution:
- If addrs4 = 1, then we will always be including loopback addresses in
the list. Since the idea of this extra condition is to exclude loopback
interfaces from the list unless they're the only available addresses, I
would suggest (addrs4 == numV4Loopback addrs6 == numV6Loopback)
instead.
- On the very limited chance that a user has a machine with only an ipv6
loopback configured (unlikely, I'd agree) it probably makes sense to
leave it in there. Actually, can you tell me why you'd rather not
include ipv6 loopbacks at all?
New webrev at:
http://cr.openjdk.java.net/~robm/7180557/webrev.04/
-Rob
On 21/09/13 12:20, Dmitry Samersoff wrote:
Rob,
See below -
ll. 210 have to be fixed, other comments is just an opinion.
Inet4AddressImpl.c:
ll.132 - it might be better to move initialization to a separate
function, the same way as in Inet6 - I really like this idea.
Inet6AddressImpl.c
ll. 126.
it's better to move static int initialized into initializeInetClasses()
and don't check it ll. 282.
ll. 170
it looks like rest of the code uses NI_MAXHOST also we have to check
results of JVM_GetHostName if it returns -1 it's probably better to
bailout immediately.
ll. 193 and below - wrong indent
4)
ll. 210
we can have more than one v4 address so
if (addrs4 = 1)
have to be here.
*.
Your include ipv6 loopback in the list if interface has no ipv4
addresses, I'm not sure this logic is correct. On my opinion it's better
to never include ipv6 loopbacks.
*.
Is it better to rename:
includeLoopback = includeLoopbacks
ll. 218
It might be better to calculate arraySize under if at ll. 210 to make
code better readable
ll. 236
It might be better to split if statement to make it more readable at the
cost of duplicating iter = iter-ifa_next; line.
e.g.
while (iter != NULL) {
...
if (family != AF_INET6 and family != AF_INET) {
iter = iter-ifa_next;
continue;
}
if ((!includeV6 and family == AF_INET6)) {
iter = iter-ifa_next;
continue;
}
if (!includeLoopback and isLoopback) {
iter = iter-ifa_next;
continue;
}
if (iter-ifa_name[0] != '\0'iter-ifa_addr) {
// Copy address to Java array
iter = iter-ifa_next;
continue; // redundant just for readability
}
}
ll.244
I'm not sure it's good to return partially complete array in case of
object allocation fail. Probably you should throw
JNU_ThrowOutOfMemoryError(env, Object allocation failed);
-Dmitry
On 2013-09-20 18:58, Rob McKenna wrote:
After a brief discussion with Chris, we decided to revert the position
of the call to lookupIfLocalAddrs so as to give the local host primacy
over DNS.
Latest (and hopefully last) webrev here:
http://cr.openjdk.java.net/~robm/7180557/webrev.03/
-Rob
On 14/09/13 00:00, Rob McKenna wrote:
Hi Bernd,
I should have said in the context of this bug. What I meant was
removing AI_CANONNAME doesn't resolve the issue as far as Mac is
concerned. I.e. I still see the UnknownHostException. In this
particular case the hostname is not set via the hosts file.
In the latest webrev the call to getifaddrs only occurs if getaddrinfo
fails.
-Rob
On 13/09/13 20:28, Bernd Eckenfels wrote:
Am 13.09.2013, 19:32 Uhr, schrieb Rob McKenna
rob.mcke...@oracle.com:
W.r.t. the use of AI_CANONNAME, this doesn't actually make a
difference in the context of this fix, but is definitely something
that should be looked at. I'll put it on the todo list.
I think it does make a difference: If you remove the CANON flag
getaddrinfo() will not do DNS lookups when the host is configured to
prefer the hosts file (which it should do on Linux and OSX). And so
the platform specific lookupIfLocalhost() can be put after the
getaddrinfo() (again).
I actually think the bug exists on all platforms. If getaddrinfo()
fails because neighter hosts nor DNS file finds the name this can
happen on all platforms. I dont think it helps to add a fallback only
on MACOSX (and there is certainly no need to prefer the fallback
then).
Gruss
Bernd
--
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: 7180557 - InetAddress.getLocalHost throws UnknownHostException on java7u5 on OSX
Rob,
Existing code uses if (JVM_GetHostName(myhostname, NI_MAXHOST)) so I
withdraw my last comments. Please, don't change it!!!
-Dmitry
On 2013-10-07 20:30, Rob McKenna wrote:
Thanks Dmitry! I'll correct that nipick pre-push.
-Rob
On 07/10/13 16:47, Dmitry Samersoff wrote:
Rob,
This version of your fix looks good for me.
Inet4AddressImpl.c:
Thumbs up.
Inet6AddressImpl.c:
Thumbs up.
173 if (JVM_GetHostName(myhostname, NI_MAXHOST)) {
Nitpicking, explicit == -1 would be better here.
Actually, can you tell me why you'd rather not
include ipv6 loopbacks at all?
If interface don't have ipv6 address but we have ipv6 loopback it most
likely means that ipv6 is not functioning properly.
-Dmitry
On 2013-10-04 19:03, Rob McKenna wrote:
Hi Dmitry,
Thanks a lot for the comprehensive review. W.r.t. line 210, I agree
there is a problem with the logic, but I'd like to suggest an
alternative solution:
- If addrs4 = 1, then we will always be including loopback addresses in
the list. Since the idea of this extra condition is to exclude loopback
interfaces from the list unless they're the only available addresses, I
would suggest (addrs4 == numV4Loopback addrs6 == numV6Loopback)
instead.
- On the very limited chance that a user has a machine with only an ipv6
loopback configured (unlikely, I'd agree) it probably makes sense to
leave it in there. Actually, can you tell me why you'd rather not
include ipv6 loopbacks at all?
New webrev at:
http://cr.openjdk.java.net/~robm/7180557/webrev.04/
-Rob
On 21/09/13 12:20, Dmitry Samersoff wrote:
Rob,
See below -
ll. 210 have to be fixed, other comments is just an opinion.
Inet4AddressImpl.c:
ll.132 - it might be better to move initialization to a separate
function, the same way as in Inet6 - I really like this idea.
Inet6AddressImpl.c
ll. 126.
it's better to move static int initialized into initializeInetClasses()
and don't check it ll. 282.
ll. 170
it looks like rest of the code uses NI_MAXHOST also we have to check
results of JVM_GetHostName if it returns -1 it's probably better to
bailout immediately.
ll. 193 and below - wrong indent
4)
ll. 210
we can have more than one v4 address so
if (addrs4 = 1)
have to be here.
*.
Your include ipv6 loopback in the list if interface has no ipv4
addresses, I'm not sure this logic is correct. On my opinion it's
better
to never include ipv6 loopbacks.
*.
Is it better to rename:
includeLoopback = includeLoopbacks
ll. 218
It might be better to calculate arraySize under if at ll. 210 to make
code better readable
ll. 236
It might be better to split if statement to make it more readable at
the
cost of duplicating iter = iter-ifa_next; line.
e.g.
while (iter != NULL) {
...
if (family != AF_INET6 and family != AF_INET) {
iter = iter-ifa_next;
continue;
}
if ((!includeV6 and family == AF_INET6)) {
iter = iter-ifa_next;
continue;
}
if (!includeLoopback and isLoopback) {
iter = iter-ifa_next;
continue;
}
if (iter-ifa_name[0] != '\0'iter-ifa_addr) {
// Copy address to Java array
iter = iter-ifa_next;
continue; // redundant just for readability
}
}
ll.244
I'm not sure it's good to return partially complete array in case of
object allocation fail. Probably you should throw
JNU_ThrowOutOfMemoryError(env, Object allocation failed);
-Dmitry
On 2013-09-20 18:58, Rob McKenna wrote:
After a brief discussion with Chris, we decided to revert the position
of the call to lookupIfLocalAddrs so as to give the local host primacy
over DNS.
Latest (and hopefully last) webrev here:
http://cr.openjdk.java.net/~robm/7180557/webrev.03/
-Rob
On 14/09/13 00:00, Rob McKenna wrote:
Hi Bernd,
I should have said in the context of this bug. What I meant was
removing AI_CANONNAME doesn't resolve the issue as far as Mac is
concerned. I.e. I still see the UnknownHostException. In this
particular case the hostname is not set via the hosts file.
In the latest webrev the call to getifaddrs only occurs if
getaddrinfo
fails.
-Rob
On 13/09/13 20:28, Bernd Eckenfels wrote:
Am 13.09.2013, 19:32 Uhr, schrieb Rob McKenna
rob.mcke...@oracle.com:
W.r.t. the use of AI_CANONNAME, this doesn't actually make a
difference in the context of this fix, but is definitely something
that should be looked at. I'll put it on the todo list.
I think it does make a difference: If you remove the CANON flag
getaddrinfo() will not do DNS lookups when the host is configured to
prefer the hosts file (which it should do on Linux and OSX). And so
the platform specific lookupIfLocalhost() can be put after the
getaddrinfo() (again).
I actually think the bug exists on all platforms. If getaddrinfo()
fails because neighter hosts nor DNS file finds the name this can
happen on all platforms. I dont
Re: JDK 8 RFR 8010371: getaddrinfo can fail with EAI_SYSTEM/EAGAIN, causes UnknownHostException to be thrown
Chris, On my opinion, it's better to just return meaningful exception to customer and let them deal with network issue (as current webrev does). Typical network failure requires at least couple of milliseconds to recover so immediate retry most probably fails with the same error. From the other side - cu has lots of possibility to defend against such failures on application level. E.g. popup a message box please check your wiring and try again In a future, it might be possible to add a timeout parameter to corresponding Java-level functions and keep trying on Java level until we get result or timeout, but it requires much more work and probably out of scope of this CR. -Dmitry On 2013-10-03 13:11, Chris Hegarty wrote: On 10/02/2013 11:18 PM, Dmitry Samersoff wrote: Chris, I'm not sure immediate native retry make sence here because tipically EAGAIN of getaddrinfo caused by network failure, like unreachable nameserver. (see my previous letter) OK, while not ideal ( please don't shoot! ) what do others think of Thread.yield() before retry. It is an attempt to allow other threads on the system do some work before us, therefore potentially swapping out our failed lookup thread until rescheduled. I'm just trying to avoid baking in a hardcoded sleep/wait value ( since we don't know what that should be ). The use of Thread.yield(), if acceptable, would of course most likely make sense to push the retry logic back up into the Java level. -Chris. -Dmitry On 2013-10-02 23:53, Chris Hegarty wrote: On 10/02/2013 08:40 PM, Brian Burkhalter wrote: So, how about this approach: 1) If the error is EAI_AGAIN / EIA_SYSTEM+EAGAIN / WSATRY_AGAIN then do one immediate native retry. 2) If the retry fails with the same error, then throw a UHE with a specific message or cause. Sounds good to me. Questions: A) Would it be better to do the retry in the Java layer, perhaps with a very short wait? native, without any wait, should be sufficient. B) Should the message or cause in #2 be explicitly document in the javadoc? I don't think it is necessary for this to be documented. It is more informational. -Chris. -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the source code.
hg: jdk8/tl/jdk: 8009213: sun/management/jdp/JdpTest.sh fails with exit code 1
Changeset: 5d6dc0cba08f Author:dsamersoff Date: 2013-10-03 16:54 +0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/5d6dc0cba08f 8009213: sun/management/jdp/JdpTest.sh fails with exit code 1 Summary: There's no guarantee that the java process has executed far enough to be found by jps when we try to obtain it's pid. Reviewed-by: sla ! test/sun/management/jdp/JdpTest.sh
Re: JDK 8 RFR 8010371: getaddrinfo can fail with EAI_SYSTEM/EAGAIN, causes UnknownHostException to be thrown
Chris, I'm not sure immediate native retry make sence here because tipically EAGAIN of getaddrinfo caused by network failure, like unreachable nameserver. (see my previous letter) -Dmitry On 2013-10-02 23:53, Chris Hegarty wrote: On 10/02/2013 08:40 PM, Brian Burkhalter wrote: So, how about this approach: 1) If the error is EAI_AGAIN / EIA_SYSTEM+EAGAIN / WSATRY_AGAIN then do one immediate native retry. 2) If the retry fails with the same error, then throw a UHE with a specific message or cause. Sounds good to me. Questions: A) Would it be better to do the retry in the Java layer, perhaps with a very short wait? native, without any wait, should be sufficient. B) Should the message or cause in #2 be explicitly document in the javadoc? I don't think it is necessary for this to be documented. It is more informational. -Chris. -- 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 : 8024952 : ClassCastException in PlainSocketImpl.accept() when using custom socketImpl
Sean,
The fix looks good for me but the code might be better readable if you
inverse the condition.
if (!(s instanceof PlainSocketImpl)) {
impl.accept(s)
return;
}
... rest of the code
-Dmitry
On 2013-10-01 18:54, Daniel Fuchs wrote:
On 10/1/13 4:50 PM, Seán Coffey wrote:
On 01/10/2013 14:51, Daniel Fuchs wrote:
On 10/1/13 3:09 PM, Seán Coffey wrote:
Since I'm only creating a dummy socketImpl to test the
classcastexception, no real networking stack is in place here. I'm
catching the NPE that would be thrown from the native
Java_java_net_TwoStacksPlainSocketImpl_socketAccept function since the
underlying socket passed to it is null.
C:\tmpjava CustomSocketImplFactory
Created Socket[addr=null,port=0,localport=0]
Exception in thread main java.lang.NullPointerException: socket is
null
at java.net.TwoStacksPlainSocketImpl.socketAccept(Native
Method)
That's what I would have expected from your previous changeset.
But you're no longer passing null - right? Now you're passing
an instance of CustomSocketImpl.
So where does the NPE come from? Could it be because you should
be calling:
ServerSocket.setSocketImplFactory(new CustomSocketImplFactory());
and not:
Socket.setSocketImplFactory(new CustomSocketImplFactory()); ?
The original bug stemmed from a custom socket Impl interacting with the
JDK ServerSocket Impl. If I move both Socket and ServerSocket factory
implementations over to use the custom Impl, the testcase doesn't get to
walk through the JDK serverSocket code and the ClassCastException is not
seen.
The NPE seen is coming from down in the native stack when the JDK
ServerSocket is running through accept request (from our client socket).
I don't think it's an issue for this case.
line 611 :
http://hg.openjdk.java.net/jdk8/tl/jdk/file/tip/src/windows/native/java/net/TwoStacksPlainSocketImpl.c
regards,
Sean.
Thanks Seán. You convinced me.
-- daniel
Or should you call both?
best regards,
-- daniel
Regards,
Sean.
Or is that going to hide future bugs?
best regards
-- daniel (not a reviewer)
--
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: JDK 8 RFR 8010371: getaddrinfo can fail with EAI_SYSTEM/EAGAIN, causes UnknownHostException to be thrown
Alan, getaddrinfo is actually a shell around couple of different calls, so for getaddrinfo EAGAIN usually means that it can't rich nameserver, not the interrupted syscall. EAI_SYSTEM means that one of underlaying calls (e.g. gethostbyname) returns an error. Under Windows, getaddrinfo never returns EAI_SYSTEM, but can set WSA code other that one returned by getaddrinfo - i.e. it's recomended to use WSAGetLastError. So I'm OK with this fix. -Dmitry On 2013-10-01 22:50, Alan Bateman wrote: On 01/10/2013 11:31, Brian Burkhalter wrote: Hello net-dev members, Please review this proposed fix at your convenience. Summary When looking up a host and an EAGAIN error is encountered, throw an instance of the new HostLookupException subclass of UnknownHostException. Issue https://bugs.openjdk.java.net/browse/JDK-8010371 Webrev http://cr.openjdk.java.net/~bpb/8010371 So is getaddrinfo returning EAGAIN or is it failing with EAI_SYSTEM and errno set to EAGAIN? I also wonder if the EAGAIN means the underlying syscall has been interrupted, in which case the normal thing to do is to retry. -Alan -- 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: 7180557 - InetAddress.getLocalHost throws UnknownHostException on java7u5 on OSX
Rob,
See below -
ll. 210 have to be fixed, other comments is just an opinion.
Inet4AddressImpl.c:
ll.132 - it might be better to move initialization to a separate
function, the same way as in Inet6 - I really like this idea.
Inet6AddressImpl.c
ll. 126.
it's better to move static int initialized into initializeInetClasses()
and don't check it ll. 282.
ll. 170
it looks like rest of the code uses NI_MAXHOST also we have to check
results of JVM_GetHostName if it returns -1 it's probably better to
bailout immediately.
ll. 193 and below - wrong indent
4)
ll. 210
we can have more than one v4 address so
if (addrs4 = 1)
have to be here.
*.
Your include ipv6 loopback in the list if interface has no ipv4
addresses, I'm not sure this logic is correct. On my opinion it's better
to never include ipv6 loopbacks.
*.
Is it better to rename:
includeLoopback = includeLoopbacks
ll. 218
It might be better to calculate arraySize under if at ll. 210 to make
code better readable
ll. 236
It might be better to split if statement to make it more readable at the
cost of duplicating iter = iter-ifa_next; line.
e.g.
while (iter != NULL) {
...
if (family != AF_INET6 and family != AF_INET) {
iter = iter-ifa_next;
continue;
}
if ((!includeV6 and family == AF_INET6)) {
iter = iter-ifa_next;
continue;
}
if (!includeLoopback and isLoopback) {
iter = iter-ifa_next;
continue;
}
if (iter-ifa_name[0] != '\0'iter-ifa_addr) {
// Copy address to Java array
iter = iter-ifa_next;
continue; // redundant just for readability
}
}
ll.244
I'm not sure it's good to return partially complete array in case of
object allocation fail. Probably you should throw
JNU_ThrowOutOfMemoryError(env, Object allocation failed);
-Dmitry
On 2013-09-20 18:58, Rob McKenna wrote:
After a brief discussion with Chris, we decided to revert the position
of the call to lookupIfLocalAddrs so as to give the local host primacy
over DNS.
Latest (and hopefully last) webrev here:
http://cr.openjdk.java.net/~robm/7180557/webrev.03/
-Rob
On 14/09/13 00:00, Rob McKenna wrote:
Hi Bernd,
I should have said in the context of this bug. What I meant was
removing AI_CANONNAME doesn't resolve the issue as far as Mac is
concerned. I.e. I still see the UnknownHostException. In this
particular case the hostname is not set via the hosts file.
In the latest webrev the call to getifaddrs only occurs if getaddrinfo
fails.
-Rob
On 13/09/13 20:28, Bernd Eckenfels wrote:
Am 13.09.2013, 19:32 Uhr, schrieb Rob McKenna rob.mcke...@oracle.com:
W.r.t. the use of AI_CANONNAME, this doesn't actually make a
difference in the context of this fix, but is definitely something
that should be looked at. I'll put it on the todo list.
I think it does make a difference: If you remove the CANON flag
getaddrinfo() will not do DNS lookups when the host is configured to
prefer the hosts file (which it should do on Linux and OSX). And so
the platform specific lookupIfLocalhost() can be put after the
getaddrinfo() (again).
I actually think the bug exists on all platforms. If getaddrinfo()
fails because neighter hosts nor DNS file finds the name this can
happen on all platforms. I dont think it helps to add a fallback only
on MACOSX (and there is certainly no need to prefer the fallback then).
Gruss
Bernd
--
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 : 8024952 : ClassCastException in PlainSocketImpl.accept() when using custom socketImpl
Sean, It might be possible to set s.fd to delegate.fd before call to impl.accept and therefore merge if instanceOf block. -Dmitry On 2013-09-20 00:21, Seán Coffey wrote: Looking for review on recently reported issue. Issue seen on windows when a custom socketImpl is in use. bug report : https://bugs.openjdk.java.net/browse/JDK-8024952 webrev : http://cr.openjdk.java.net/~coffeys/webrev.8024952/webrev/ Regards, Sean. -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the source code.
Re: RFR:JDK-8021372 NetworkInterface.getNetworkInterfaces() returns duplicate hardware address
Mark,
Looks good for me besides the fact that
if (((ptr-IfIndex != 0)(ptr-IfIndex == index)) ||
((ptr-Ipv6IfIndex !=0) (ptr-Ipv6IfIndex == index))) {
could be written as:
ll. 129
if ( index == 0 )
return NULL;
if (ptr-IfIndex == index ||
ptr-Ipv6IfIndex == index) {
...
}
But I'm OK to leave everything as is - compiler should do it for you.
-Dmitry
On 2013-09-05 20:07, Mark Sheppard wrote:
Hi,
please oblige and review the latest version of the fix below to address-
the issue in JDK-8021372:
NetworkInterface.getNetworkInterfaces() returns duplicate hardware address
This incorporates Chris' suggestions wrt amending the associated test.
http://cr.openjdk.java.net/~msheppar/8021372/webrev.02/
regards
Mark
On 04/09/2013 11:39, Chris Hegarty wrote:
Mark,
The source changes look good to me.
The test is a good addition, but trivially I would...
1) remove the @build and @run tags. I don't think they are needed
as I don't see that the test needs to run in othervm mode.
2) remove all exception handling code, and just declare all methods
to throw Exception. The jtreg harness will show the test as failed
if an unhandled exception is thrown.
-Chris.
On 04/09/2013 08:59, Mark Sheppard wrote:
Hi
based on feedback for initial fix, an amended webrev has been created.
So, please oblige and review the fix below to address the issue in
JDK-8021372: NetworkInterface.getNetworkInterfaces() returns duplicate
hardware address
webrev:
http://cr.openjdk.java.net/~msheppar/8021372/webrev.01/
regressions re-run with no ill effects
regards
Mark
--
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: 7180557 - InetAddress.getLocalHost throws UnknownHostException on java7u5 on OSX
Rob, Did you try to remove hints.ai_flags = AI_CANONNAME this flag asks getaddreinfo to return FQDN, but the function behavior is not clearly defined for the case where FQDN is not available. -Dmitry On 2013-09-03 19:18, Rob McKenna wrote: Hi folks, Mac seems to have trouble looking up local hostnames using getaddrinfo unless a domain is set. The solution is to add a check with getifaddrs . This fix replaces a usage of _ALLBSD_SOURCE with MACOSX. I haven't seen a canonical answer on whether this is the way to go so I figured trial by fire might get the discussion going. http://cr.openjdk.java.net/~robm/7180557/webrev.01/ -Rob -- 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: 7180557 - InetAddress.getLocalHost throws UnknownHostException on java7u5 on OSX
Rob, Instead of iterating interfaces list twice you can maintain two indices, as you already know exact number of addresses. put_v4_at(i) put_v6_at(j+v4count) Also you can take a look at solaris/native/java/net/NetworkInterface.c and possibly re-use some logic from there. -Dmitry On 2013-09-03 19:18, Rob McKenna wrote: Hi folks, Mac seems to have trouble looking up local hostnames using getaddrinfo unless a domain is set. The solution is to add a check with getifaddrs . This fix replaces a usage of _ALLBSD_SOURCE with MACOSX. I haven't seen a canonical answer on whether this is the way to go so I figured trial by fire might get the discussion going. http://cr.openjdk.java.net/~robm/7180557/webrev.01/ -Rob -- 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:JDK-8021372 NetworkInterface.getNetworkInterfaces() returns duplicate hardware address
Mark,
1. If I read the code correctly,
ll. 168 - 177 is exactly the same as ll. 180 - 189
Could you refactor the fix to avoid code duplication?
Would something like
if (
(ptr-IfIndex != 0 ptr-IfIndex == index) ||
(ptr-Ipv6IfIndex != 0 ptr-Ipv6IfIndex == index)
) {
...
}
work?
2. It seems to me passed index couldn't be 0 so null check is redundant,
but I don't mind to keep it.
It might make sense to explicitly check passed index for 0 above loop.
-Dmitry
On 2013-09-03 13:23, Mark Sheppard wrote:
Hi
please oblige and review the fix below to address the issue in JDK-8021372:
NetworkInterface.getNetworkInterfaces() returns duplicate hardware address
http://cr.openjdk.java.net/~msheppar/8021372/webrev/
the handling of the Ipv6IfIndex was suspect when setting the
interface index and when retrieving the mac address using the index.
in the getAdpaters function, the conditaionalstatement could be rolled
into one.
But, as there was a spurious IPv4 comment, I structured it as an if
else. statement.
It should be noted that IfIndex and the Ipv6IfIndex can be zero, which
implies that the
IPv4 or IPv6 interface is not available. This may be at odds with the
setting of a default index to 0 in the Java NetworkInterface abstraction,
where a defaultIndex of zero is used.
JPRTs have been run and show no side effects.
regards
Mark
--
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: Code review request, 8020842 IDN do not throw IAE when hostname ends with a trailing dot
1:35 AM, Michael McMahon wrote: I don't really understand the reason for the restriction in SNIHostName But, I guess that is where it should be enforced if it is required. Michael. On 06/08/13 17:43, Dmitry Samersoff wrote: Xuelei, . (dot) is perfectly valid domain name and it means root domain so com. is valid domain name as well. It thinks to me that in context of methods your change we should ignore trailing dots, rather than throw exception. -Dmitry On 2013-08-06 15:44, Xuelei Fan wrote: Hi, Please review the bug fix to strict the illegal input checking in IDN. webrev: http://cr.openjdk.java.net./~xuelei/8020842/webrev.00/ Here is two test cases, which are expected to get IAE. Case 1: String host = IDN.toASCII(., IDN.USE_STD3_ASCII_RULES); Exception in thread main java.lang.StringIndexOutOfBoundsException: String index out of range: 0 at java.lang.StringBuffer.charAt(StringBuffer.java:204) at java.net.IDN.toASCIIInternal(IDN.java:279) at java.net.IDN.toASCII(IDN.java:118) Case 2: String host = IDN.toASCII(com., IDN.USE_STD3_ASCII_RULES); Thanks, Xuelei -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the source code.
Re: Code review request, 8020842 IDN do not throw IAE when hostname ends with a trailing dot
Xuelei, root label is an empty label[1], dot is a label separator, so in printed form domain names is dot-terminated. Please see also below inline. [1] RFC rfc1034.txt: Internally, programs that manipulate domain names should represent them as sequences of labels, where each label is a length octet followed by an octet string. Because all domain names end at the root, *which has a null string for a label*, these internal representations can use a length byte of zero to terminate a domain name. On 2013-08-07 16:44, Xuelei Fan wrote: On 8/7/2013 12:06 AM, Matthew Hall wrote: Trailing dots are allowed in plain DNS (thus almost surely in IDN), and the single dot represents the root zone. So you have to be careful making this sort of change to check the DNS RFCs first. That's the first question we need to answer, whether IDN allow tailling dots (com.), zero-length root label (.), and zero-length label (, for example example..com)? Per the specification of IDN.toASCII(): === ToASCII operation can fail. ToASCII fails if any step of it fails. If ToASCII operation fails, an IllegalArgumentException will be thrown. In this case, the input string should not be used in an internationalized domain name. A label is an individual part of a domain name. The original ToASCII operation, as defined in RFC 3490, only operates on a single label. This method can handle both label and entire domain name, by assuming that labels in a domain name are always separated by dots. ... Throws IllegalArgumentException - if the input string doesn't conform to RFC 3490 specification Per the specification of RFC 3490: == [section 2] A label is an individual part of a domain name. Labels are usually shown separated by dots; for example, the domain name www.example.com is composed of three labels: www, example, and com. (The zero-length root label described in [STD13], which can be explicit as in www.example.com. or implicit as in www.example.com, is not considered a label in this specification.) An internationalized label is a label to which the ToASCII operation (see section 4) can be applied without failing (with the UseSTD3ASCIIRules flag unset). ... Although most Unicode characters can appear in internationalized labels, ToASCII will fail for some input strings, and such strings are not valid internationalized labels. An internationalized domain name (IDN) is a domain name in which every label is an internationalized label. [Section 4.1] ToASCII consists of the following steps: ... 8. Verify that the number of code points is in the range 1 to 63 inclusive. Here are the questions: 1. whether example..com is an valid IDN? As dot is used as label separators, there are three labels, example, , com. Per RFC 3490, is not a valid label. Hence, example..com is not a valid IDN. We need to address the issue in IDN. Root label can't appear in the middle of domain name, so example..com is an invalid domain name and appropriate exception have to be thrown. 2. whether xyz. is an valid IDN? It's an gray area, I think. We can treat the trailing . as root label, or a label separator. If the trailing . is treated as label separator, xyz. is invalid per RFC 3490. if the trailing . is treated as root label, what's the expected return value of IDN.toASCII(xyz.)? I think the return value can be either xyz. or xyz. The current implementation returns xyz. We may need not to update the implementation if tailing . is treated as root label. Empty label at the end of domain names is valid per RFC 1034 and means root label. So we should process this name and return all non-empty labels. 3. whether . is an valid IDN? It's an gray area again, I think. As above, if the trailing . is treated as root label, I think the return value can be either . or . The current implementation throws a StringIndexOutOfBoundsException. However, what empty domain name () really means? I would prefer to return . for . instead. We need to address the issue in IDN. As dot is a label separator and root (empty) label can't appear in the middle of domain name, . (dot) is not valid name and this case is similar to case (1) - we should throw an appropriate exception. -Dmitry Here comes the solution, the IDN.toASCII() returns: 1. . for .; 2. xyz for xyz.; 3. IAE for example..com. Does it make sense? Thanks, Xuelei On 8/7/2013 1:35 AM, Michael McMahon wrote: I don't really understand the reason for the restriction in SNIHostName But, I guess that is where it should be enforced if it is required. Michael. On 06/08/13 17:43, Dmitry Samersoff wrote: Xuelei, . (dot) is perfectly valid domain name and it means root domain so com. is valid domain name as well. It thinks to me that in context of methods your change we should ignore
hg: jdk8/tl/jdk: 8011038: sourceObj validation during desereliazation of RelationNotification should be relaxed
Changeset: fce446b29577 Author:dsamersoff Date: 2013-08-06 14:04 +0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/fce446b29577 8011038: sourceObj validation during desereliazation of RelationNotification should be relaxed Summary: sourceObj could be set to null by setSource() relax a validation of deserialized object. Reviewed-by: sjiang, skoivu, dfuchs ! src/share/classes/javax/management/relation/RelationNotification.java
Re: Code review request, 8020842 IDN do not throw IAE when hostname ends with a trailing dot
Xuelei, . (dot) is perfectly valid domain name and it means root domain so com. is valid domain name as well. It thinks to me that in context of methods your change we should ignore trailing dots, rather than throw exception. -Dmitry On 2013-08-06 15:44, Xuelei Fan wrote: Hi, Please review the bug fix to strict the illegal input checking in IDN. webrev: http://cr.openjdk.java.net./~xuelei/8020842/webrev.00/ Here is two test cases, which are expected to get IAE. Case 1: String host = IDN.toASCII(., IDN.USE_STD3_ASCII_RULES); Exception in thread main java.lang.StringIndexOutOfBoundsException: String index out of range: 0 at java.lang.StringBuffer.charAt(StringBuffer.java:204) at java.net.IDN.toASCIIInternal(IDN.java:279) at java.net.IDN.toASCII(IDN.java:118) Case 2: String host = IDN.toASCII(com., IDN.USE_STD3_ASCII_RULES); Thanks, Xuelei -- 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: Code Review Request: 8020498: Crash when both libnet.so and libmawt.so are loaded
Kurchi, As you don't plan to use this function outside of net_util.c it's better to add static keyword as well. -Dmitry On 2013-07-18 21:55, Kurchi Hazra wrote: Hi Chris, Thanks for the additional information. How about this fix: http://cr.openjdk.java.net/~khazra/8020498/webrev.01/ - Kurchi On 7/18/2013 2:44 AM, Chris Hegarty wrote: On 18/07/2013 01:19, Kurchi Hazra wrote: Hi, We are seeing a crash when both libmawt.so and libnet.so are loaded, and the init() method in src/share/native/java/net/net_util.c is called, but an init() method in libmawt.so gets invoked instead. Although it is difficult to reproduce the problem, declaring the init() in net_util.c can resolve the issue. Thanks Kurchi, I have seen this, what appears to be a bizarre linking issue, a few times now over the past year but was unable to get to the bottom of it. Snipped of a stacktrace and comment from 8005450 ( I was unable to get the bottom of it, or reproduce ). --- This looks like a strange linking issue. Relevant part of the stack below. ... C [libmawt.so+0x26c12] init+0x8e;; init+0x8e C [libnet.so+0x8b53] NET_SockaddrToInetAddress+0x23;; NET_SockaddrToInetAddress+0x23 C [libnet.so+0xc388] Java_java_net_PlainSocketImpl_socketAccept+0x41c;; Java_java_net_PlainSocketImpl_socketAccept+0x41c j java.net.PlainSocketImpl.socketAccept(Ljava/net/SocketImpl;)V+0 ... NET_SockaddrToInetAddress is somehow triggering libmawt init() to be called. NET_SockaddrToInetAddress does call a local init() function, that is defined within net_util.c, but it is as if the linker has decided to call libmawt init(). --- I think your fix is probably ok, but I wonder if we should air on the side of caution and maybe give this function a unique name, say initInetAddrs ( or something similar ). -Chris. This low-risk fix will hopefully be going into 7u40. Bug: http://bugs.sun.com/view_bug.do?bug_id=8020498 Webrev: http://cr.openjdk.java.net/~khazra/8020498/webrev.00/ Thanks, Kurchi -- 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 JDK8015799
Chris,
Looks good for me.
Thank you for doing it.
-Dmitry
On 2013-06-28 13:36, Chris Hegarty wrote:
The latest webrev is
http://cr.openjdk.java.net/~jzavgren/8015799/webrev.03/
We end up with:
private String filterHeaderField(String name, String value) {
if (value == null)
return null;
if (SET_COOKIE.equalsIgnoreCase(name) ||
SET_COOKIE2.equalsIgnoreCase(name)) {
// Filtering only if there is a cookie handler. [Assumption:
the
// cookie handler will store/retrieve the HttpOnly cookies]
if (cookieHandler == null || value.length() == 0)
return value;
So return value is not changed. BTW. I agree with your comments.
-Chris.
On 06/27/2013 06:13 PM, Dmitry Samersoff wrote:
Chris,
1. I'm not sure it's a correct to return null rather then empty value,
but you understand better what is happening, so I'm leaving it up
to you.
2. It might be better to move
2805 if (value == null)
2806 return null;
under if(SET_COOKIE ...), i.e. to ll. 2810
it doesn't change anything in practice - the methods continue returning
null for all cases where value is null - but makes code better
understandable.
-Dmitry
On 2013-06-27 00:49, Chris Hegarty wrote:
To link this email thread, both in the archives, and for others. The
call for review on this bug started with:
http://mail.openjdk.java.net/pipermail/net-dev/2013-June/006607.html
On 06/26/2013 08:22 PM, Kurchi Hazra wrote:
On 6/26/2013 12:17 PM, Kurchi Hazra wrote:
Hi John,
Why not change lines 2810-2811 to:
if (value == null || value.length() == 0)
return value;
I meant return null. For other cookie-headers too, is there any reason
for us not returning null if the length of value is 0?
In the first webrev John had made this change, but I asked him to revert
it and only change the Set-Cookie(2) headers.
Since all header retrieval passes through filterHeaderField, in one way
or another, I'm a little concerned about changing this. Also, as the
only issue we know of is with Set-Cookie(2), maybe you could add the
empty string check to these headers only? ( that is to say, move the
'value.length() == 0' check into the ' if
(SET_COOKIE.equalsIgnoreCase(name). '
The difference is, currently if a header value is non-null and has a
length of 0 ( i.e. empty string ), then empty string is returned. With
the original change then null is returned.
We have been bitten by subtle changes in this area before. Returning
null from such an API, URLConnection.getHeaderField(s), for cases where
it did not return null before may lead to NPE's in some applications.
-Chris.
Also, lots of formatting issue in the test, especially in
TestCookieHandler, try-catch block indentation is off in line 54.
Its also best to stop the server in a finally clause at line 58.
Alternatively, I also liked Andreas' use of autocloseable in his test
for 6563286. See [1].
Yes, please.
-Chris.
- Kurchi
[1]
http://cr.openjdk.java.net/~arieber/6563286/webrev.00/test/sun/net/www/http/HttpURLConnection/MalformedFollowRedirect.java.html
http://cr.openjdk.java.net/%7Earieber/6563286/webrev.00/test/sun/net/www/http/HttpURLConnection/MalformedFollowRedirect.java.html
On 6/26/2013 10:54 AM, John Zavgren wrote:
Please consider the following changes to the Java cookie code.
http://cr.openjdk.java.net/~jzavgren/8015799/webrev.02/
http://cr.openjdk.java.net/%7Ejzavgren/8015799/webrev.02/
The problem I fixed occurs when a server returns an array of cookies
that contains a null cookie.
Thanks
John
--
John Zavgren
john.zavg...@oracle.com
603-821-0904
US-Burlington-MA
--
-Kurchi
--
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 JDK8015799
Chris, 1. I'm not sure it's a correct to return null rather then empty value, but you understand better what is happening, so I'm leaving it up to you. 2. It might be better to move 2805 if (value == null) 2806 return null; under if(SET_COOKIE ...), i.e. to ll. 2810 it doesn't change anything in practice - the methods continue returning null for all cases where value is null - but makes code better understandable. -Dmitry On 2013-06-27 00:49, Chris Hegarty wrote: To link this email thread, both in the archives, and for others. The call for review on this bug started with: http://mail.openjdk.java.net/pipermail/net-dev/2013-June/006607.html On 06/26/2013 08:22 PM, Kurchi Hazra wrote: On 6/26/2013 12:17 PM, Kurchi Hazra wrote: Hi John, Why not change lines 2810-2811 to: if (value == null || value.length() == 0) return value; I meant return null. For other cookie-headers too, is there any reason for us not returning null if the length of value is 0? In the first webrev John had made this change, but I asked him to revert it and only change the Set-Cookie(2) headers. Since all header retrieval passes through filterHeaderField, in one way or another, I'm a little concerned about changing this. Also, as the only issue we know of is with Set-Cookie(2), maybe you could add the empty string check to these headers only? ( that is to say, move the 'value.length() == 0' check into the ' if (SET_COOKIE.equalsIgnoreCase(name). ' The difference is, currently if a header value is non-null and has a length of 0 ( i.e. empty string ), then empty string is returned. With the original change then null is returned. We have been bitten by subtle changes in this area before. Returning null from such an API, URLConnection.getHeaderField(s), for cases where it did not return null before may lead to NPE's in some applications. -Chris. Also, lots of formatting issue in the test, especially in TestCookieHandler, try-catch block indentation is off in line 54. Its also best to stop the server in a finally clause at line 58. Alternatively, I also liked Andreas' use of autocloseable in his test for 6563286. See [1]. Yes, please. -Chris. - Kurchi [1] http://cr.openjdk.java.net/~arieber/6563286/webrev.00/test/sun/net/www/http/HttpURLConnection/MalformedFollowRedirect.java.html http://cr.openjdk.java.net/%7Earieber/6563286/webrev.00/test/sun/net/www/http/HttpURLConnection/MalformedFollowRedirect.java.html On 6/26/2013 10:54 AM, John Zavgren wrote: Please consider the following changes to the Java cookie code. http://cr.openjdk.java.net/~jzavgren/8015799/webrev.02/ http://cr.openjdk.java.net/%7Ejzavgren/8015799/webrev.02/ The problem I fixed occurs when a server returns an array of cookies that contains a null cookie. Thanks John -- John Zavgren john.zavg...@oracle.com 603-821-0904 US-Burlington-MA -- -Kurchi -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
hg: jdk8/tl/jdk: 8015604: JDP packets containing ideographic characters are broken
Changeset: df1b35c7901d Author:dsamersoff Date: 2013-06-05 18:20 +0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/df1b35c7901d 8015604: JDP packets containing ideographic characters are broken Summary: Code uses string length rather than byte array length and non ascii entry brakes packet. Reviewed-by: dholmes, jbachorik, sla ! src/share/classes/sun/management/jdp/JdpPacketWriter.java ! test/sun/management/jdp/JdpUnitTest.java
hg: jdk8/tl/jdk: 8014420: Default JDP address does not match the one assigned by IANA
Changeset: 7d7bfce34a79 Author:dsamersoff Date: 2013-05-28 18:46 +0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/7d7bfce34a79 8014420: Default JDP address does not match the one assigned by IANA Summary: JDP protocol defaults changed to IANA assigned values Reviewed-by: dholmes, jbachorik, hirt Contributed-by: fwei...@redhat.com ! src/share/classes/sun/management/Agent.java ! src/share/classes/sun/management/jdp/package-info.java ! test/sun/management/jdp/JdpTest.sh
Re: Code review: 8010464: Evolve java networking same origin policy
Michael,
It might be better to narrow permissions right now with code like below:
private static
AccessControlContext withPermissions(Permission ... perms){
Permissions col = new Permissions();
for (Permission thePerm : perms ) {
col.add(thePerm);
}
final ProtectionDomain pd = new ProtectionDomain(null, col);
return new AccessControlContext( new ProtectionDomain[] { pd });
}
AccessController.doPrivileged(
new PrivilegedExceptionActionVoid() {
public Void run() throws IOException {
plainConnect0();
return null;
}, withPermissions(p)
);
-Dmitry
On 2013-05-10 15:34, Michael McMahon wrote:
Hi,
This is the webrev for the HttpURLPermission addition.
As well as the new permission class, the change
includes the use of the permission in java.net.HttpURLConnection.
The code basically checks for a HttpURLPermission in plainConnect(),
getInputStream() and getOutputStream() for the request and if
the caller has permission the request is executed in a doPrivileged()
block. When the limited doPrivileged feature is integrated, I will
change the doPrivileged() call to limit the privilege elevation to a single
SocketPermission (as shown in the code comments).
The webrev is at http://cr.openjdk.java.net/~michaelm/8010464/webrev.1/
Thanks
Michael
--
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the source code.
Re: suggestions for improvement in java.net APIs
Alan, Not sure what is real usecase for this requirements for ipv4. For ipv6 it should be done by sendmsg() and msg_control. -Dmitry On 2013-05-06 11:28, Alan Bateman wrote: On 05/05/2013 12:19, Dmitry Samersoff wrote: Alan, SO_BINDTODEVICE shouldn't be used in modern applications because it causes more problems than solves. e.g. prevents application from handling on-fly device changes. The use-case is where an application wants to control the network or source address. -Alan. -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * Give Rabbit time, and he'll always get the answer
Re: suggestions for improvement in java.net APIs
Alan, SO_BINDTODEVICE shouldn't be used in modern applications because it causes more problems than solves. e.g. prevents application from handling on-fly device changes. Also it requires root (or be more precise RAW_SOCKET) permission and is not supported on some embedded platforms. -Dmitry On 2013-05-05 13:40, Alan Bateman wrote: On 04/05/2013 13:22, Miika Komu wrote: : Multihoming bug --- * R3.2: Server-side multihoming for UDP does not work properly. The framework should use SO_BINDTODEVICE option or sendmsg()/recvmsg() interfaces in a proper way. Thanks for sending the link to the survey. On SO_BINDTODEVICE, then I think the last time that this came up (a few years ago) that the issue was that it wasn't supported by many operating systems and on Linux I thought it required privileges/root. On sendmsg/recvmsg then there was an effort at one point to add support for looking at the destination address, I don't recall if setting the source address when sending was considered at the time. If you are interested in working on adding this support then it would be good to propose a patch. The main challenges be implementing it on all platforms or specifying in such a way it allows for some implementation-specific behavior/support. Suggested improvements (for better end-user or developer experience) * R2.2: The framework does not support parallel DNS look ups over IPv4 and IPv6 to optimize latency InetAddress will normally delegate to the platform's resolver so it might be parallel (newer versions of glibc? Also I think MacOSX has been doing this for some time). InetAddress can also be configured to use a pure-DNS or other provider so that the look ups can be controlled. I'm not aware of anyone looking into doing parallel look-ups over IPv4 and IPv6 but it would be an interesting patch for someone to work on if they are interested. * R3.3: The framework does not support parallel connect() over IPv4 and IPv6 to minimize the latency for connection set-up While there isn't API support, it is something that can be done at the application level. So if InetAddress.getAllByName return several addresses then it is possible to initiate connections to several addresses using a thread pool. Another approach might be to use several SocketChannel configured non-blocking plus a Selector. This may not be what you are thinking of course. -Alan. -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * Give Rabbit time, and he'll always get the answer
Re: API change for 8010464: Evolve java networking same origin policy
Michael, GET,POST:Header1,Header2 Colon is a delimiter between http header and it's value. With this syntax we might have problems in a future if sometimes we will support different headers for different methods or add an ability to check header value as well. -Dmitry On 2013-04-30 14:30, Michael McMahon wrote: Hi Kurchi, I can include such an example easily. Eg: GET,POST:Header1,Header2 means one permission that permits either GET or POST with either or both of the two headers. If you wanted to restrict one set of headers to GET and another set to POST, then that would require two different permissions. - Michael On 30/04/13 00:40, Kurchi Hazra wrote: Hi Michael, From the documentation, it is not clear to me how to represent both request-headers and method list together in an actions string for two or more methods. (Say I have two methods GET and POST and I want to specify a request-headers list for each, how do I do it?) Maybe another example will help. Thanks, Kurchi On 4/29/2013 3:53 AM, Michael McMahon wrote: On 28/04/13 09:01, Chris Hegarty wrote: In the main I link the new HttpURLPermission class. When reading the docs I found the references to the URL and URL string confusing ( it could be just me ). When I see capital 'URL' my mind instantly, and incorrectly, goes to java.net.URL. In all cases you mean the URL string given when constructing the HttpURLPermission, right? Yes, that is what is meant. The class does not use j.n.URL at all, as that would bring us back to the old (undesirable) behavior with DNS lookups required for basic operations like equals() and hashCode() Another example is the equals method Returns true if, this.getActions().equals(p.getActions()) and p's URL equals this's URL. Returns false otherwise. this is referring so a simple string comparison of the given URL string, right? This should be case insensitive too. Does it take into account default protocol ports, e.g. http://foo.com/ equal http://foo.com:80/ The implementation uses a java.net.URI internally. So URI takes care of that. implies() makes reference to the URL scheme, and other specific parts of the URL. Also, the constructors throw IAE 'if url is not a valid URL', but what does this mean. Should we just bite the bullet and just say that URI is used to parse the given string into its specific parts? Otherwise, how can this be validated. I originally didn't want to mention URI in the apidoc due to potential confusion surrounding the use of URL in the permission class name. But, maybe it would be clearer to be explicit about it, particularly for the equals() behavior. Otherwise we have to specify all of it in this class. As for the additions to HttpURLConnection, what are the implications on proxies? Permissions, etc... There's no change in behavior with respect to proxies. Permission is given to connect to proxies implicitly except in cases where the caller specifies the proxy through the URL.openConnection(Proxy) api. There are other unusual cases like the Http Use Proxy response. Explicit permission is required for that case also. Thanks! Michael -Chris. On 04/26/2013 03:36 PM, Michael McMahon wrote: Hi, The is the suggested API for one of the two new JEPs recently submitted. This is for JEP 184: HTTP URL Permissions The idea here is to define a higher level http permission class which knows about URLs, HTTP request methods and headers. So, it is no longer necessary to grant blanket permission for any kind of TCP connection to a host/port. Instead a HttpURLPermission restricts access to only the Http protocol itself. Restrictions can also be imposed based on URL paths, specific request methods and request headers. The API change can be seen at the URL below: http://cr.openjdk.java.net/~michaelm/8010464/api/ In addition to defining a new permission class, HttpURLConnection is modified to make use of it and the documentation change describing this can be seen at the link below: http://cr.openjdk.java.net/~michaelm/8010464/api/blender.html All comments welcome. Thanks Michael. -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * Give Rabbit time, and he'll always get the answer
Re: API change for 8010464: Evolve java networking same origin policy
Michael,
I'm just asking about replacing : (colon) to another character to be
able to write something like:
permission
java.net.HttpURLPermission http://www.foo.com/-;,
GET Location: http://www.foo.com/*, Content-type: image/jpeg;
in a future
-Dmitry.
On 2013-05-01 15:04, Michael McMahon wrote:
On 01/05/13 11:09, Dmitry Samersoff wrote:
Michael,
GET,POST:Header1,Header2
Colon is a delimiter between http header and it's value.
With this syntax we might have problems in a future if sometimes we will
support different headers for different methods or add an ability to
check header value as well.
-Dmitry
Dmitry,
It would complicate the syntax a lot if you wanted to support
different headers for different methods. Would be a lot simpler
to just grant separate permissions for the two cases. Eg.
grant {
permission java.net.HttpURLPermission http://www.foo.com/-;,
GET:Header1,Header2;
permission java.net.HttpURLPermission http://www.foo.com/-;,
POST:Header3,Header4;
};
Michael
On 2013-04-30 14:30, Michael McMahon wrote:
Hi Kurchi,
I can include such an example easily. Eg:
GET,POST:Header1,Header2
means one permission that permits either GET or POST with either or both
of the two headers. If you wanted to restrict one set of headers to GET
and another set to POST, then that would require two different
permissions.
- Michael
On 30/04/13 00:40, Kurchi Hazra wrote:
Hi Michael,
From the documentation, it is not clear to me how to represent
both request-headers and method list together in an actions string for
two or more methods. (Say I have two methods GET and POST and I want
to specify a request-headers list for each, how do I do it?) Maybe
another example will help.
Thanks,
Kurchi
On 4/29/2013 3:53 AM, Michael McMahon wrote:
On 28/04/13 09:01, Chris Hegarty wrote:
In the main I link the new HttpURLPermission class.
When reading the docs I found the references to the URL and URL
string confusing ( it could be just me ). When I see capital 'URL'
my mind instantly, and incorrectly, goes to java.net.URL. In all
cases you mean the URL string given when constructing the
HttpURLPermission, right?
Yes, that is what is meant. The class does not use j.n.URL at all, as
that would bring us back
to the old (undesirable) behavior with DNS lookups required for basic
operations like equals() and hashCode()
Another example is the equals method
Returns true if, this.getActions().equals(p.getActions()) and p's
URL equals this's URL. Returns false otherwise.
this is referring so a simple string comparison of the given URL
string, right? This should be case insensitive too. Does it take
into account default protocol ports, e.g. http://foo.com/ equal
http://foo.com:80/
The implementation uses a java.net.URI internally. So URI takes care
of that.
implies() makes reference to the URL scheme, and other specific
parts of the URL. Also, the constructors throw IAE 'if url is not a
valid URL', but what does this mean. Should we just bite the bullet
and just say that URI is used to parse the given string into its
specific parts? Otherwise, how can this be validated.
I originally didn't want to mention URI in the apidoc due to
potential confusion surrounding the use of URL in the permission
class name. But, maybe it would be clearer to be explicit about it,
particularly for the equals() behavior.
Otherwise we have to specify all of it in this class.
As for the additions to HttpURLConnection, what are the implications
on proxies? Permissions, etc...
There's no change in behavior with respect to proxies. Permission is
given to connect to proxies implicitly
except in cases where the caller specifies the proxy through the
URL.openConnection(Proxy) api.
There are other unusual cases like the Http Use Proxy response.
Explicit permission is required
for that case also.
Thanks!
Michael
-Chris.
On 04/26/2013 03:36 PM, Michael McMahon wrote:
Hi,
The is the suggested API for one of the two new JEPs recently
submitted.
This is for JEP 184: HTTP URL Permissions
The idea here is to define a higher level http permission class
which knows about URLs, HTTP request methods and headers.
So, it is no longer necessary to grant blanket permission for any
kind
of TCP connection to a host/port. Instead a HttpURLPermission
restricts
access to only the Http protocol itself. Restrictions can also be
imposed
based on URL paths, specific request methods and request headers.
The API change can be seen at the URL below:
http://cr.openjdk.java.net/~michaelm/8010464/api/
In addition to defining a new permission class, HttpURLConnection
is modified to make use of it and the documentation change
describing this
can be seen at the link below:
http://cr.openjdk.java.net/~michaelm/8010464/api/blender.html
All comments welcome.
Thanks
Michael.
--
Dmitry Samersoff
Oracle Java development team, Saint
Re: API change for 8010464: Evolve java networking same origin policy
Michael,
Sorry for not being clean enough.
On my opinion an ability to check header value as well as a header name
is quite useful future for the real world.
e.g. to being able to prevent redirection to other domain or limit
certain content type etc.
I understand, that these changes is out of scope of today work, but it
quite possible that we implement it sometimes in a future.
For (header-name: header-value) pair : (colon) is a native delimiter,
so it's better not to use it to separate methods list and headers list.
On my opinion, (space) is enough for this case and better reflect HTTP
header i.e.
GET,POST Header1,Header2
-Dmitry
On 2013-05-01 15:16, Michael McMahon wrote:
Ah right. The permission only contains header names.
It never contains header values. And header names are tokens
in the Http spec that cannot contain a colon character.
Michael
On 01/05/13 12:11, Dmitry Samersoff wrote:
Michael,
I'm just asking about replacing : (colon) to another character to be
able to write something like:
permission
java.net.HttpURLPermission http://www.foo.com/-;,
GET Location: http://www.foo.com/*, Content-type: image/jpeg;
in a future
-Dmitry.
On 2013-05-01 15:04, Michael McMahon wrote:
On 01/05/13 11:09, Dmitry Samersoff wrote:
Michael,
GET,POST:Header1,Header2
Colon is a delimiter between http header and it's value.
With this syntax we might have problems in a future if sometimes we
will
support different headers for different methods or add an ability to
check header value as well.
-Dmitry
Dmitry,
It would complicate the syntax a lot if you wanted to support
different headers for different methods. Would be a lot simpler
to just grant separate permissions for the two cases. Eg.
grant {
permission java.net.HttpURLPermission http://www.foo.com/-;,
GET:Header1,Header2;
permission java.net.HttpURLPermission http://www.foo.com/-;,
POST:Header3,Header4;
};
Michael
On 2013-04-30 14:30, Michael McMahon wrote:
Hi Kurchi,
I can include such an example easily. Eg:
GET,POST:Header1,Header2
means one permission that permits either GET or POST with either or
both
of the two headers. If you wanted to restrict one set of headers to
GET
and another set to POST, then that would require two different
permissions.
- Michael
On 30/04/13 00:40, Kurchi Hazra wrote:
Hi Michael,
From the documentation, it is not clear to me how to represent
both request-headers and method list together in an actions string
for
two or more methods. (Say I have two methods GET and POST and I want
to specify a request-headers list for each, how do I do it?) Maybe
another example will help.
Thanks,
Kurchi
On 4/29/2013 3:53 AM, Michael McMahon wrote:
On 28/04/13 09:01, Chris Hegarty wrote:
In the main I link the new HttpURLPermission class.
When reading the docs I found the references to the URL and URL
string confusing ( it could be just me ). When I see capital 'URL'
my mind instantly, and incorrectly, goes to java.net.URL. In all
cases you mean the URL string given when constructing the
HttpURLPermission, right?
Yes, that is what is meant. The class does not use j.n.URL at
all, as
that would bring us back
to the old (undesirable) behavior with DNS lookups required for
basic
operations like equals() and hashCode()
Another example is the equals method
Returns true if, this.getActions().equals(p.getActions())
and p's
URL equals this's URL. Returns false otherwise.
this is referring so a simple string comparison of the given URL
string, right? This should be case insensitive too. Does it take
into account default protocol ports, e.g. http://foo.com/ equal
http://foo.com:80/
The implementation uses a java.net.URI internally. So URI takes care
of that.
implies() makes reference to the URL scheme, and other specific
parts of the URL. Also, the constructors throw IAE 'if url is
not a
valid URL', but what does this mean. Should we just bite the bullet
and just say that URI is used to parse the given string into its
specific parts? Otherwise, how can this be validated.
I originally didn't want to mention URI in the apidoc due to
potential confusion surrounding the use of URL in the permission
class name. But, maybe it would be clearer to be explicit about it,
particularly for the equals() behavior.
Otherwise we have to specify all of it in this class.
As for the additions to HttpURLConnection, what are the
implications
on proxies? Permissions, etc...
There's no change in behavior with respect to proxies. Permission is
given to connect to proxies implicitly
except in cases where the caller specifies the proxy through the
URL.openConnection(Proxy) api.
There are other unusual cases like the Http Use Proxy response.
Explicit permission is required
for that case also.
Thanks!
Michael
-Chris.
On 04/26/2013 03:36 PM, Michael McMahon wrote:
Hi,
The is the suggested API for one
Re: API change for 8010464: Evolve java networking same origin policy
Michael,
On 2013-05-01 16:40, Michael McMahon wrote:
Another possibility if we were to do that in the future, could be:
GET,POST:Header1=value 1,Header2=value 2
Variant with space looks more natural for me as it follows HTTP header
syntax.
But it's up to you to make a decision.
-Dmitry
- Michael
On 01/05/13 12:38, Dmitry Samersoff wrote:
Michael,
Sorry for not being clean enough.
On my opinion an ability to check header value as well as a header name
is quite useful future for the real world.
e.g. to being able to prevent redirection to other domain or limit
certain content type etc.
I understand, that these changes is out of scope of today work, but it
quite possible that we implement it sometimes in a future.
For (header-name: header-value) pair : (colon) is a native delimiter,
so it's better not to use it to separate methods list and headers list.
On my opinion, (space) is enough for this case and better reflect HTTP
header i.e.
GET,POST Header1,Header2
-Dmitry
On 2013-05-01 15:16, Michael McMahon wrote:
Ah right. The permission only contains header names.
It never contains header values. And header names are tokens
in the Http spec that cannot contain a colon character.
Michael
On 01/05/13 12:11, Dmitry Samersoff wrote:
Michael,
I'm just asking about replacing : (colon) to another character to be
able to write something like:
permission
java.net.HttpURLPermission http://www.foo.com/-;,
GET Location: http://www.foo.com/*, Content-type: image/jpeg;
in a future
-Dmitry.
On 2013-05-01 15:04, Michael McMahon wrote:
On 01/05/13 11:09, Dmitry Samersoff wrote:
Michael,
GET,POST:Header1,Header2
Colon is a delimiter between http header and it's value.
With this syntax we might have problems in a future if sometimes we
will
support different headers for different methods or add an ability to
check header value as well.
-Dmitry
Dmitry,
It would complicate the syntax a lot if you wanted to support
different headers for different methods. Would be a lot simpler
to just grant separate permissions for the two cases. Eg.
grant {
permission java.net.HttpURLPermission http://www.foo.com/-;,
GET:Header1,Header2;
permission java.net.HttpURLPermission http://www.foo.com/-;,
POST:Header3,Header4;
};
Michael
On 2013-04-30 14:30, Michael McMahon wrote:
Hi Kurchi,
I can include such an example easily. Eg:
GET,POST:Header1,Header2
means one permission that permits either GET or POST with either or
both
of the two headers. If you wanted to restrict one set of headers to
GET
and another set to POST, then that would require two different
permissions.
- Michael
On 30/04/13 00:40, Kurchi Hazra wrote:
Hi Michael,
From the documentation, it is not clear to me how to
represent
both request-headers and method list together in an actions string
for
two or more methods. (Say I have two methods GET and POST and I
want
to specify a request-headers list for each, how do I do it?) Maybe
another example will help.
Thanks,
Kurchi
On 4/29/2013 3:53 AM, Michael McMahon wrote:
On 28/04/13 09:01, Chris Hegarty wrote:
In the main I link the new HttpURLPermission class.
When reading the docs I found the references to the URL and
URL
string confusing ( it could be just me ). When I see capital
'URL'
my mind instantly, and incorrectly, goes to java.net.URL. In all
cases you mean the URL string given when constructing the
HttpURLPermission, right?
Yes, that is what is meant. The class does not use j.n.URL at
all, as
that would bring us back
to the old (undesirable) behavior with DNS lookups required for
basic
operations like equals() and hashCode()
Another example is the equals method
Returns true if, this.getActions().equals(p.getActions())
and p's
URL equals this's URL. Returns false otherwise.
this is referring so a simple string comparison of the given URL
string, right? This should be case insensitive too. Does it take
into account default protocol ports, e.g. http://foo.com/ equal
http://foo.com:80/
The implementation uses a java.net.URI internally. So URI takes
care
of that.
implies() makes reference to the URL scheme, and other specific
parts of the URL. Also, the constructors throw IAE 'if url is
not a
valid URL', but what does this mean. Should we just bite the
bullet
and just say that URI is used to parse the given string into its
specific parts? Otherwise, how can this be validated.
I originally didn't want to mention URI in the apidoc due to
potential confusion surrounding the use of URL in the permission
class name. But, maybe it would be clearer to be explicit about
it,
particularly for the equals() behavior.
Otherwise we have to specify all of it in this class.
As for the additions to HttpURLConnection, what are the
implications
on proxies? Permissions, etc...
There's no change in behavior with respect to proxies
Re: RFR-JDK8012108
John,
Looks good for me besides some nits:
NetworkInterface.c:
132,208,405: better rearrange if and get rid of extra else
151: occasional space changes
162: //else comment seems redundant for me
408: missed free(tableP)
NetworkInterface_winXP.c:
103: else is redundant
141: { bracket misplaced
146: //else comment seems redundant for me
ResolverConfigurationImpl.c:
130: better rearrange if and get rid of extra else
free(adapterP) missed
-Dmitry
On 2013-04-25 05:10, John Zavgren wrote:
All:
I expanded the scope of the work for this bug and cleaned up other
realloc() errors in the windows native code. I believe I've identified
all unsafe calls to realloc() in this corner of the native code.
Two additional files were affected.
Please let me know what you think:
http://cr.openjdk.java.net/~jzavgren/8012108/webrev.03/
http://cr.openjdk.java.net/%7Ejzavgren/8012108/webrev.03/
Thanks!
John
On 04/20/2013 10:36 AM, Kurchi Subhra Hazra wrote:
On Apr 20, 2013, at 4:40 AM, Dmitry Samersoff dmitry.samers...@oracle.com
wrote:
Kurchi,
if *netaddrPP == NULL at 367 and calloc fails at 391 we would jump
to freeAllocatedMemory with start == NULL
True, but then we skip to line 444 since *netaddrPP == NULL, so we don't get
to line 438.
I am just saying it is not strictly necessary to check if start is null
before entering the first if block. We might want to guard against how the
code changes in future and put in the check though.
I have no ideas whether this code path is possible in reality or not,
but it stops my eyes.
-Dmitry
On 2013-04-20 14:55, Kurchi Subhra Hazra wrote:
On Apr 20, 2013, at 3:16 AM, Dmitry Samersoff
dmitry.samers...@oracle.com wrote:
John,
102, 145 else is redundant.
438 - we will crash here if start is null
Maybe it is good to have an additional check for start != null, but from
what I see, start will not be null if *netaddrPP is not null.
-Dmitry
On 2013-04-20 01:33, John Zavgren wrote:
Greetings:
I fixed the bad realloc pattern. Please let me know what you think.
http://cr.openjdk.java.net/~jzavgren/8012108/webrev.02/
Thanks!
John Z
- Original Message -
From: chris.hega...@oracle.com
To: net-dev@openjdk.java.net, john.zavg...@oracle.com
Cc: dmitry.samers...@oracle.com
Sent: Friday, April 19, 2013 8:59:25 AM GMT -05:00 US/Canada Eastern
Subject: Re: RFR-JDK8012108
On 18/04/2013 22:11, Dmitry Samersoff wrote:
John,
I see bad realloc pattern here. Could you fix it as well?
Yes, please. Otherwise the changes look fine.
-Chris.
e.g.
93 adapterInfo = (IP_ADAPTER_ADDRESSES *) realloc (adapterInfo,
len);
-Dmitry
On 2013-04-19 00:56, John Zavgren wrote:
Greetings:
I fixed a case in the windows native code where calloc() was being used
without checking it's returned value.
http://cr.openjdk.java.net/~jzavgren/8012108/webrev.01/
Thanks!
John Zavgren
--
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* Give Rabbit time, and he'll always get the answer
--
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* Give Rabbit time, and he'll always get the answer
--
John Zavgren
john.zavg...@oracle.com
603-821-0904
US-Burlington-MA
--
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* Give Rabbit time, and he'll always get the answer
Re: RFR-JDK8012108
John,
OK. Thank you for the explanation.
It looks like i misread the code.
-Dmitry
On 2013-04-24 19:35, John Zavgren wrote:
Dmitry:
Thanks for your comments
From my reading of the code for the procedure: getAddrsFromAdapter(...):
If *netaddrPP == NULL at line 369, and a jump is made to
freeAllocatedMemory because of a calloc() failure, then obviously the
assignment operation on line 429 (*netaddrPP = start) is skipped, and
*netaddrPP remains NULL... and consequently the block:
if (*netaddrPP != NULL) {
// We started with an existing list
curr=start-next;
start-next = NULL;
start = curr;
}
is not executed (and no references are made to start-next).
On the other hand, if *netaddrPP == NULL at line 369 and no errors
occur, we make the assignment:
*netaddrPP = start at line 429 and return immediately (without
considering start-next).
If you see any errors in my thinking, please let me know.
Thanks!
John
On 04/20/2013 07:40 AM, Dmitry Samersoff wrote:
Kurchi,
if *netaddrPP == NULL at 367 and calloc fails at 391 we would jump
to freeAllocatedMemory with start == NULL
I have no ideas whether this code path is possible in reality or not,
but it stops my eyes.
-Dmitry
On 2013-04-20 14:55, Kurchi Subhra Hazra wrote:
On Apr 20, 2013, at 3:16 AM, Dmitry Samersoff
dmitry.samers...@oracle.com wrote:
John,
102, 145 else is redundant.
438 - we will crash here if start is null
Maybe it is good to have an additional check for start != null, but
from what I see, start will not be null if *netaddrPP is not null.
-Dmitry
On 2013-04-20 01:33, John Zavgren wrote:
Greetings:
I fixed the bad realloc pattern. Please let me know what you think.
http://cr.openjdk.java.net/~jzavgren/8012108/webrev.02/
Thanks!
John Z
- Original Message -
From: chris.hega...@oracle.com
To: net-dev@openjdk.java.net, john.zavg...@oracle.com
Cc: dmitry.samers...@oracle.com
Sent: Friday, April 19, 2013 8:59:25 AM GMT -05:00 US/Canada Eastern
Subject: Re: RFR-JDK8012108
On 18/04/2013 22:11, Dmitry Samersoff wrote:
John,
I see bad realloc pattern here. Could you fix it as well?
Yes, please. Otherwise the changes look fine.
-Chris.
e.g.
93 adapterInfo = (IP_ADAPTER_ADDRESSES *) realloc
(adapterInfo, len);
-Dmitry
On 2013-04-19 00:56, John Zavgren wrote:
Greetings:
I fixed a case in the windows native code where calloc() was
being used
without checking it's returned value.
http://cr.openjdk.java.net/~jzavgren/8012108/webrev.01/
Thanks!
John Zavgren
--
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* Give Rabbit time, and he'll always get the answer
--
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* Give Rabbit time, and he'll always get the answer
Re: RFR-JDK8012108
Kurchi, if *netaddrPP == NULL at 367 and calloc fails at 391 we would jump to freeAllocatedMemory with start == NULL I have no ideas whether this code path is possible in reality or not, but it stops my eyes. -Dmitry On 2013-04-20 14:55, Kurchi Subhra Hazra wrote: On Apr 20, 2013, at 3:16 AM, Dmitry Samersoff dmitry.samers...@oracle.com wrote: John, 102, 145 else is redundant. 438 - we will crash here if start is null Maybe it is good to have an additional check for start != null, but from what I see, start will not be null if *netaddrPP is not null. -Dmitry On 2013-04-20 01:33, John Zavgren wrote: Greetings: I fixed the bad realloc pattern. Please let me know what you think. http://cr.openjdk.java.net/~jzavgren/8012108/webrev.02/ Thanks! John Z - Original Message - From: chris.hega...@oracle.com To: net-dev@openjdk.java.net, john.zavg...@oracle.com Cc: dmitry.samers...@oracle.com Sent: Friday, April 19, 2013 8:59:25 AM GMT -05:00 US/Canada Eastern Subject: Re: RFR-JDK8012108 On 18/04/2013 22:11, Dmitry Samersoff wrote: John, I see bad realloc pattern here. Could you fix it as well? Yes, please. Otherwise the changes look fine. -Chris. e.g. 93 adapterInfo = (IP_ADAPTER_ADDRESSES *) realloc (adapterInfo, len); -Dmitry On 2013-04-19 00:56, John Zavgren wrote: Greetings: I fixed a case in the windows native code where calloc() was being used without checking it's returned value. http://cr.openjdk.java.net/~jzavgren/8012108/webrev.01/ Thanks! John Zavgren -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * Give Rabbit time, and he'll always get the answer -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * Give Rabbit time, and he'll always get the answer
Re: RFR 8012244: java/net/Socket/asyncClose/Race.java fails intermittently on Windows
Chris, src/windows/native/java/net/SocketInputStream.c:137 May be we can just check for WSA_INVALID_HANDLE here -Dmitry On 2013-04-15 21:08, Chris Hegarty wrote: Test issue where the test fails on some Windows versions, and minor implementation clean up. http://cr.openjdk.java.net/~chegar/8012244/webrev.00/webrev/ -Chris. -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * Give Rabbit time, and he'll always get the answer
Re: Request for review: 8009650 - HttpClient available() check throws SocketException when connection has been closed
Rob,
Sorry for not being clean enough. We have repeated pattern:
if (logger.isLoggable(PlatformLogger.FINEST)) {
logger.finest(HttpClient.available(): + msg
}
so it makes code better readable if we can put it to some common place.
-Dmitry
On 2013-03-08 02:31, Rob McKenna wrote:
Hi Dmitry,
I'm not 100% sure what you mean by duplication, the exceptions and their
messages are distinct. I think it would be best to keep it that way.
-Rob
On 07/03/13 22:00, Dmitry Samersoff wrote:
Rob,
Is it possible to avoid code duplication?
i.e. do something like this:
int r;
try {
...
} catch (SocketException e) {
// Comments goes here
r = -1
}
if (r == -1){
if (logger. ...
available = false;
}
return available;
-Dmitry
On 2013-03-07 20:18, Rob McKenna wrote:
Hi folks,
This is a slight alteration of the fix contributed by Stuart Douglas.
This fix deals with a SocketException caused by getSoTimeout() on a
closed connection.
http://cr.openjdk.java.net/~robm/8009650/webrev.01/
-Rob
--
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* Give Rabbit time, and he'll always get the answer
Re: RFR JDK-8008804
John, closesocket call should be here. -Dmitry On 2013-03-04 20:12, John Zavgren wrote: Greetings: I posted a webrev image for a modification that eliminates a file descriptor leak. http://cr.openjdk.java.net/~jzavgren/8008804/webrev.01/ Thanks! John -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * Give Rabbit time, and he'll always get the answer
hg: jdk8/tl/jdk: 8008095: TEST_BUG: JDK-8002048 one more testcase failure on Solaris
Changeset: 8181be9a3538 Author:dsamersoff Date: 2013-02-13 21:06 +0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/8181be9a3538 8008095: TEST_BUG: JDK-8002048 one more testcase failure on Solaris Summary: fixed couple of more Solaris shell incompatibilities Reviewed-by: chegar ! test/sun/management/jdp/JdpTest.sh
hg: jdk8/tl/jdk: 8007536: Incorrect copyright header in JDP files
Changeset: 1df991184045 Author:dsamersoff Date: 2013-02-11 18:44 +0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/1df991184045 8007536: Incorrect copyright header in JDP files Summary: Copyright header in JDP files missed the classpath exception rule. Reviewed-by: mikael ! src/share/classes/sun/management/jdp/JdpBroadcaster.java ! src/share/classes/sun/management/jdp/JdpController.java ! src/share/classes/sun/management/jdp/JdpException.java ! src/share/classes/sun/management/jdp/JdpGenericPacket.java ! src/share/classes/sun/management/jdp/JdpJmxPacket.java ! src/share/classes/sun/management/jdp/JdpPacket.java ! src/share/classes/sun/management/jdp/JdpPacketReader.java ! src/share/classes/sun/management/jdp/JdpPacketWriter.java ! src/share/classes/sun/management/jdp/package-info.java
hg: jdk8/tl/jdk: 8007277: JDK-8002048 testcase fails to compile
Changeset: 0e7d5dd84fdf Author:dsamersoff Date: 2013-02-06 16:53 +0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/0e7d5dd84fdf 8007277: JDK-8002048 testcase fails to compile Summary: sun.* classes is not included to ct.sym file and symbol file have to be ignored Reviewed-by: alanb ! test/sun/management/jdp/JdpTest.sh
hg: jdk8/tl/jdk: 8002048: Protocol to discovery of manageable Java processes on a network
Changeset: 962d6612cace Author:dsamersoff Date: 2013-02-03 21:39 +0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/962d6612cace 8002048: Protocol to discovery of manageable Java processes on a network Summary: Introduce a protocol to discover manageble Java instances across a network subnet, JDP Reviewed-by: sla, dfuchs ! src/share/classes/sun/management/Agent.java + src/share/classes/sun/management/jdp/JdpBroadcaster.java + src/share/classes/sun/management/jdp/JdpController.java + src/share/classes/sun/management/jdp/JdpException.java + src/share/classes/sun/management/jdp/JdpGenericPacket.java + src/share/classes/sun/management/jdp/JdpJmxPacket.java + src/share/classes/sun/management/jdp/JdpPacket.java + src/share/classes/sun/management/jdp/JdpPacketReader.java + src/share/classes/sun/management/jdp/JdpPacketWriter.java + src/share/classes/sun/management/jdp/package-info.java + test/sun/management/jdp/JdpClient.java + test/sun/management/jdp/JdpDoSomething.java + test/sun/management/jdp/JdpTest.sh + test/sun/management/jdp/JdpUnitTest.java
Re: RFR 8006395: Race in async socket close on Linux
Chris, Looks good for me. -Dmitry On 2013-01-31 01:36, Chris Hegarty wrote: There is a very small, and very old, window for a race in the socket asynchronous close mechanism on Linux. This can lead to blocking socket operations never returning even after the socket has been closed. This issue would appear to exist since its (linux_close.c) creation back in 1.4, but since the window for the race is tiny, it seems to have gone unnoticed until now. It was originally diagnosed through code inspection, but since then I have created and added a small test that reproduce the issue about one in every 10 - 20 runs, with jdk8, on Ubuntu 12.04, with 2x 2.33GHz Intel Xeon E5345 (2x quad-core, 1 thread per core = 8 threads). closefd first interrupts (sends wakeup signal to) all the threads blocked on the fd, then it closes/dup2's the fd. However, the signal may arrive at its target thread before that thread has entered the blocking system call, and before close/dup2. In this case, the target thread will simple enter the blocking system call and never return. Solution - If it was to close/dup2 the fd before issuing the wake up, then any thread not yet blocked in a system call should see that the fd is closed on entry, otherwise it will be woken up by the signal. While there is an equivalent closefd in bsd_close.c ( mac/bsd specific code), I have not been able to reproduce this issue after many test runs on mac. Also, making similar changes to closefd in bsd_close runs into a problem with dup2; dup2 will hang if another thread is doing a blocking operation. I believe this issue is similar to 7133499. So as far as this issue is concerned changes will only be make to the Linux version of closefd. Webrev --- http://cr.openjdk.java.net/~chegar/8006395/webrev.00/webrev/ -Chris. -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * Give Rabbit time, and he'll always get the answer
Re: RFR JDK-8005120
be (hopefully) a system call error. Thanks! John Zavgren http://cr.openjdk.java.net/~mullan/webrevs/jzavgren/8005120/ -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * Give Rabbit time, and he'll always get the answer
Re: RFR JDK-8005120
John, socket_md.c:88,99,110 etc. could you change int len to size_t len as well? -Dmitry On 2012-12-21 02:47, John Zavgren wrote: Greetings: I modified my changes so that windows knows the definition of the POSIX data type: socklen_t, and now all the system calls are using the doctrinaire data types. Please consider the following update. http://cr.openjdk.java.net/~mullan/webrevs/jzavgren/8005120/webrev.01/ Thanks! John Zavgren On 20/12/2012 13:49, John Zavgren wrote: Greetings: I agree that the correct way to fix this problem is to use POSIX data types, e.g., socklen_t. However, when I switch to the doctrinaire data type, the build fails on windows machines: - build monologue - c:\jprt\t\p1\032220.jzavgren\s\jdk\src\share\transport\socket\sysSocket.h(39) : error C2146: syntax error : missing ')' before identifier 'len' c:\jprt\t\p1\032220.jzavgren\s\jdk\src\share\transport\socket\sysSocket.h(39) : error C2081: 'socklen_t' : name in formal parameter list illegal c:\jprt\t\p1\032220.jzavgren\s\jdk\src\share\transport\socket\sysSocket.h(39) : error C2061: syntax error : identifier 'len' c:\jprt\t\p1\032220.jzavgren\s\jdk\src\share\transport\socket\sysSocket.h(39) : error C2059: syntax error : ';' c:\jprt\t\p1\032220.jzavgren\s\jdk\src\share\transport\socket\sysSocket.h(39) : error C2059: syntax error : ')' - build monologue - I used alternative types, e.g., uint32_t, etc. as a way to avoid the limitations of windows. What is the recommended way to accommodate this windows limitation? Shall I use a typedef statement to define socklen_t? We don't suffer from this issue in the networking native code. The unix and windows implementations are distinct. I see the vm defines socklen_t in a windows specific header, hotspot/src/os/windows/vm/jvm_windows.h, as typedef int socklen_t; ...and it is then used in shared code, like jvm.cpp, and the hpi, by optionally including #ifdef TARGET_OS_FAMILY_windows # include jvm_windows.h #endif We could use a similar, but more simplistic, approach here. -Chris. Thanks! - Original Message - From: chris.hega...@oracle.com To: david.hol...@oracle.com Cc: alan.bate...@oracle.com, serviceability-...@openjdk.java.net, john.zavg...@oracle.com, net-dev@openjdk.java.net Sent: Thursday, December 20, 2012 7:41:07 AM GMT -05:00 US/Canada Eastern Subject: Re: RFR JDK-8005120 On 19/12/2012 20:52, David Holmes wrote: Real sense of deja-vu here. Didn't we go through this same thing with the HPI socket routines? Yes, and the networking native code too. I think it is best to use socklen_t for the unix code. From what I can see making these changes, to use socklen_t, should be relatively localized. -Chris. Depending on the OS (and version?) we should be using socklen_t not int and not uint32_t. David On 20/12/2012 2:35 AM, Chris Hegarty wrote: John, I grabbed your patch, and with it I now see different warnings. ../../../../src/share/transport/socket/socketTransport.c: In function 'socketTransport_startListening': ../../../../src/share/transport/socket/socketTransport.c:310:40: warning: pointer targets in passing argument 3 of 'dbgsysGetSocketName' differ in signedness [-Wpointer-sign] ../../../../src/share/transport/socket/sysSocket.h:58:5: note: expected 'uint32_t *' but argument is of type 'int *' ../../../../src/share/transport/socket/socketTransport.c: In function 'socketTransport_accept': ../../../../src/share/transport/socket/socketTransport.c:371:33: warning: pointer targets in passing argument 3 of 'dbgsysAccept' differ in signedness [-Wpointer-sign] ../../../../src/share/transport/socket/sysSocket.h:41:5: note: expected 'uint32_t *' but argument is of type 'int *' Do you see these in your build? -Chris. On 12/19/2012 03:42 PM, Alan Bateman wrote: John - this is the debugger socket transport so cc'ing the serviceability-dev list as that is where this code is maintained. On 19/12/2012 15:36, John Zavgren wrote: Greetings: Please consider the following change to the two files: src/share/transport/socket/sysSocket.h src/solaris/transport/socket/socket_md.c that eliminate compiler warnings that stem from the fact that the variables that the native code passes to various system calls were not declared correctly. They were declared as integers, but they must be unsigned integers because they are used to define buffer lengths. Were one to supply a negative value as an argument, it would be cast into an unsigned Martian value and there'd be (hopefully) a system call error. Thanks! John Zavgren http://cr.openjdk.java.net/~mullan/webrevs/jzavgren/8005120/ -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * Give Rabbit time, and he'll always get the answer
hg: jdk8/tl/jdk: 7009998: JMX synchronization during connection restart is faulty
Changeset: 3f014bc09297 Author:dsamersoff Date: 2012-12-20 17:24 +0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3f014bc09297 7009998: JMX synchronization during connection restart is faulty Summary: add a return statement after the re-connecting has finished and the state is CONNECTED Reviewed-by: sjiang Contributed-by: jaroslav.bacho...@oracle.com ! make/netbeans/jmx/build.properties ! src/share/classes/com/sun/jmx/remote/internal/ClientCommunicatorAdmin.java
Re: RFR JDK-8005120
John, On windows socklen_t defined in WS2TCPIP.H make sure it's included when necessary. -Dmitry On 2012-12-20 17:49, John Zavgren wrote: Greetings: I agree that the correct way to fix this problem is to use POSIX data types, e.g., socklen_t. However, when I switch to the doctrinaire data type, the build fails on windows machines: - build monologue - c:\jprt\t\p1\032220.jzavgren\s\jdk\src\share\transport\socket\sysSocket.h(39) : error C2146: syntax error : missing ')' before identifier 'len' c:\jprt\t\p1\032220.jzavgren\s\jdk\src\share\transport\socket\sysSocket.h(39) : error C2081: 'socklen_t' : name in formal parameter list illegal c:\jprt\t\p1\032220.jzavgren\s\jdk\src\share\transport\socket\sysSocket.h(39) : error C2061: syntax error : identifier 'len' c:\jprt\t\p1\032220.jzavgren\s\jdk\src\share\transport\socket\sysSocket.h(39) : error C2059: syntax error : ';' c:\jprt\t\p1\032220.jzavgren\s\jdk\src\share\transport\socket\sysSocket.h(39) : error C2059: syntax error : ')' - build monologue - I used alternative types, e.g., uint32_t, etc. as a way to avoid the limitations of windows. What is the recommended way to accommodate this windows limitation? Shall I use a typedef statement to define socklen_t? Thanks! - Original Message - From: chris.hega...@oracle.com To: david.hol...@oracle.com Cc: alan.bate...@oracle.com, serviceability-...@openjdk.java.net, john.zavg...@oracle.com, net-dev@openjdk.java.net Sent: Thursday, December 20, 2012 7:41:07 AM GMT -05:00 US/Canada Eastern Subject: Re: RFR JDK-8005120 On 19/12/2012 20:52, David Holmes wrote: Real sense of deja-vu here. Didn't we go through this same thing with the HPI socket routines? Yes, and the networking native code too. I think it is best to use socklen_t for the unix code. From what I can see making these changes, to use socklen_t, should be relatively localized. -Chris. Depending on the OS (and version?) we should be using socklen_t not int and not uint32_t. David On 20/12/2012 2:35 AM, Chris Hegarty wrote: John, I grabbed your patch, and with it I now see different warnings. ../../../../src/share/transport/socket/socketTransport.c: In function 'socketTransport_startListening': ../../../../src/share/transport/socket/socketTransport.c:310:40: warning: pointer targets in passing argument 3 of 'dbgsysGetSocketName' differ in signedness [-Wpointer-sign] ../../../../src/share/transport/socket/sysSocket.h:58:5: note: expected 'uint32_t *' but argument is of type 'int *' ../../../../src/share/transport/socket/socketTransport.c: In function 'socketTransport_accept': ../../../../src/share/transport/socket/socketTransport.c:371:33: warning: pointer targets in passing argument 3 of 'dbgsysAccept' differ in signedness [-Wpointer-sign] ../../../../src/share/transport/socket/sysSocket.h:41:5: note: expected 'uint32_t *' but argument is of type 'int *' Do you see these in your build? -Chris. On 12/19/2012 03:42 PM, Alan Bateman wrote: John - this is the debugger socket transport so cc'ing the serviceability-dev list as that is where this code is maintained. On 19/12/2012 15:36, John Zavgren wrote: Greetings: Please consider the following change to the two files: src/share/transport/socket/sysSocket.h src/solaris/transport/socket/socket_md.c that eliminate compiler warnings that stem from the fact that the variables that the native code passes to various system calls were not declared correctly. They were declared as integers, but they must be unsigned integers because they are used to define buffer lengths. Were one to supply a negative value as an argument, it would be cast into an unsigned Martian value and there'd be (hopefully) a system call error. Thanks! John Zavgren http://cr.openjdk.java.net/~mullan/webrevs/jzavgren/8005120/ -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * Give Rabbit time, and he'll always get the answer
hg: jdk8/tl/jdk: 8005309: Missed tests for 6783290,6937053,7009998
Changeset: c1a55ee9618e Author:dsamersoff Date: 2012-12-20 20:12 +0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/c1a55ee9618e 8005309: Missed tests for 6783290,6937053,7009998 Summary: Missed tests for 6783290,6937053,7009998 Reviewed-by: sjiang, emcmanus Contributed-by: jaroslav.bacho...@oracle.com + test/com/sun/jmx/remote/CCAdminReconnectTest.java + test/com/sun/jmx/remote/NotificationMarshalVersions/Client/Client.java + test/com/sun/jmx/remote/NotificationMarshalVersions/Client/ConfigKey.java + test/com/sun/jmx/remote/NotificationMarshalVersions/Client/TestNotification.java + test/com/sun/jmx/remote/NotificationMarshalVersions/Server/ConfigKey.java + test/com/sun/jmx/remote/NotificationMarshalVersions/Server/Server.java + test/com/sun/jmx/remote/NotificationMarshalVersions/Server/Ste.java + test/com/sun/jmx/remote/NotificationMarshalVersions/Server/SteMBean.java + test/com/sun/jmx/remote/NotificationMarshalVersions/Server/TestNotification.java + test/com/sun/jmx/remote/NotificationMarshalVersions/TestSerializationMismatch.sh + test/javax/management/MBeanInfo/SerializationTest1.java
Re: RFR JDK-8005120
John, Did you consider using socklen_t instead of uint32_t and unsigned int (for namelen etc) ? -Dmitry On 2012-12-19 19:36, John Zavgren wrote: Greetings: Please consider the following change to the two files: src/share/transport/socket/sysSocket.h src/solaris/transport/socket/socket_md.c that eliminate compiler warnings that stem from the fact that the variables that the native code passes to various system calls were not declared correctly. They were declared as integers, but they must be unsigned integers because they are used to define buffer lengths. Were one to supply a negative value as an argument, it would be cast into an unsigned Martian value and there'd be (hopefully) a system call error. Thanks! John Zavgren http://cr.openjdk.java.net/~mullan/webrevs/jzavgren/8005120/ -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * Give Rabbit time, and he'll always get the answer
Re: RFR JDK-8005120
John,
On 2012-12-19 22:25, John Zavgren wrote:
Yes... I did consider that, but I didn't see any POSIX data types near the
code I was changing, so I decided to use a brute force data type instead.
connect, recvfrom etc almost always uses socklen_t, but socklen_t could
expands either to int or to unsigned it on different OS
so as soon as the code pass a parameter directly to one of such
functions like e. g.
52 dbgsysConnect(int fd, struct sockaddr *name, uint32_t namelen) {
53 int rv = connect(fd, name, namelen);
on my opinion, we should use socklen_t
-Dmitry
Shall I make that change?
John
- Original Message -
From: dmitry.samers...@oracle.com
To: john.zavg...@oracle.com
Cc: net-dev@openjdk.java.net
Sent: Wednesday, December 19, 2012 1:06:52 PM GMT -05:00 US/Canada Eastern
Subject: Re: RFR JDK-8005120
John,
Did you consider using socklen_t instead of uint32_t and unsigned int
(for namelen etc) ?
-Dmitry
On 2012-12-19 19:36, John Zavgren wrote:
Greetings:
Please consider the following change to the two files:
src/share/transport/socket/sysSocket.h
src/solaris/transport/socket/socket_md.c
that eliminate compiler warnings that stem from the fact that the variables
that the native code passes to various system calls were not declared
correctly. They were declared as integers, but they must be unsigned
integers because they are used to define buffer lengths. Were one to supply
a negative value as an argument, it would be cast into an unsigned Martian
value and there'd be (hopefully) a system call error.
Thanks!
John Zavgren
http://cr.openjdk.java.net/~mullan/webrevs/jzavgren/8005120/
--
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* Give Rabbit time, and he'll always get the answer
Re: RFR JDK-8005120
John,
On 2012-12-19 23:33, John Zavgren wrote:
Dmitry:
Are you suggesting that in the definition of, for example, dbgsysConnect(),
we convert the namelen argument to be a socklen_t?
The best approach IMHO, is
dbgsysConnect(int fd, struct sockaddr *name, socklen_t namelen)
and change
share/transport/socket/socketTransport.c:462
err = dbgsysConnect(socketFD, (struct sockaddr *)sa, sizeof(sa));
to
err = dbgsysConnect(socketFD, (struct sockaddr *)sa,
(socklen_t) sizeof(sa));
-Dmitry
int
dbgsysConnect(int fd, struct sockaddr *name, uint32_t namelen) {
int rv = connect(fd, name, (socklen_t)nameLength);--- NEW
if (rv 0 (errno == EINPROGRESS || errno == EINTR)) {
return DBG_EINPROGRESS;
} else {
return rv;
}
}
Thanks!
John
- Original Message -
From: dmitry.samers...@oracle.com
To: john.zavg...@oracle.com
Cc: net-dev@openjdk.java.net
Sent: Wednesday, December 19, 2012 1:40:11 PM GMT -05:00 US/Canada Eastern
Subject: Re: RFR JDK-8005120
John,
On 2012-12-19 22:25, John Zavgren wrote:
Yes... I did consider that, but I didn't see any POSIX data types near the
code I was changing, so I decided to use a brute force data type instead.
connect, recvfrom etc almost always uses socklen_t, but socklen_t could
expands either to int or to unsigned it on different OS
so as soon as the code pass a parameter directly to one of such
functions like e. g.
52 dbgsysConnect(int fd, struct sockaddr *name, uint32_t namelen) {
53 int rv = connect(fd, name, namelen);
on my opinion, we should use socklen_t
-Dmitry
Shall I make that change?
John
- Original Message -
From: dmitry.samers...@oracle.com
To: john.zavg...@oracle.com
Cc: net-dev@openjdk.java.net
Sent: Wednesday, December 19, 2012 1:06:52 PM GMT -05:00 US/Canada Eastern
Subject: Re: RFR JDK-8005120
John,
Did you consider using socklen_t instead of uint32_t and unsigned int
(for namelen etc) ?
-Dmitry
On 2012-12-19 19:36, John Zavgren wrote:
Greetings:
Please consider the following change to the two files:
src/share/transport/socket/sysSocket.h
src/solaris/transport/socket/socket_md.c
that eliminate compiler warnings that stem from the fact that the variables
that the native code passes to various system calls were not declared
correctly. They were declared as integers, but they must be unsigned
integers because they are used to define buffer lengths. Were one to supply
a negative value as an argument, it would be cast into an unsigned
Martian value and there'd be (hopefully) a system call error.
Thanks!
John Zavgren
http://cr.openjdk.java.net/~mullan/webrevs/jzavgren/8005120/
--
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* Give Rabbit time, and he'll always get the answer
Re: RFR 8004925: java/net/Socks/SocksV4Test.java failing on all platforms
Chris, According to rfc2606 TLD .invalid is reserved for cases like this one, So, it seems to me domainame.invalid is the best approach. -Dmitry On 2012-12-12 20:15, Chris Hegarty wrote: On 12/12/2012 14:14, Alan Bateman wrote: -Chris. Would it be better if the test SocksServer had a list of knows that it always rejects? That might speed up the test too as it would avoid is trying to resolve host names or connect to hosts that don't exist. The UHE is thrown from the client socket connect(). The Server in this case doesn't ever receive the destination address or host name. It is simply replying to the initial/opening SOCKS handshake. The updated host name is still brittle ( if a .t TLD is ever registered! ). I don't have a better alternative. -Chris. -Alan -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * Give Rabbit time, and he'll always get the answer
Re: RFR 8004925: java/net/Socks/SocksV4Test.java failing on all platforms
On 2012-12-12 22:29, Chris Hegarty wrote: On 12/12/2012 18:15, Dmitry Samersoff wrote: Chris, According to rfc2606 TLD .invalid is reserved for cases like this one, Yes, I came across this, but there is nothing to stop an internal DNS server from resolving .invalid domains. Anyway, may doesnot.exist.invalid would be sufficient. You can't prevent internal DNS from resolving anything without doing some heavy tricks, so I guess doesnot.exist.invalid and error message that clear states that DNS setup violates rfc2606 is sufficient. -Dmitry -Chris. So, it seems to me domainame.invalid is the best approach. -Dmitry On 2012-12-12 20:15, Chris Hegarty wrote: On 12/12/2012 14:14, Alan Bateman wrote: -Chris. Would it be better if the test SocksServer had a list of knows that it always rejects? That might speed up the test too as it would avoid is trying to resolve host names or connect to hosts that don't exist. The UHE is thrown from the client socket connect(). The Server in this case doesn't ever receive the destination address or host name. It is simply replying to the initial/opening SOCKS handshake. The updated host name is still brittle ( if a .t TLD is ever registered! ). I don't have a better alternative. -Chris. -Alan -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * Give Rabbit time, and he'll always get the answer
Re: Request for code review 6512101: Incorrect encoding in NetworkInterface.getDisplayName()
Frank, Excellent! Thank you for doing it. -Dmitry On 2012-12-10 12:00, Frank Ding wrote: Hi Dmitry, I updated wording accordingly @ http://cr.openjdk.java.net/~dingxmin/6512101/webrev.03. It is now changed to Cannot get multibyte char for interface display name. What about that? Best regards, Frank On 12/10/2012 3:43 PM, Dmitry Samersoff wrote: Frank, Looks good for me. May be Can't get WIDE string should be changed to something more verbose. -Dmitry On 2012-12-10 11:40, Frank Ding wrote: Hi Dmitry and Chris, Could you please review the second revision again? Thanks and Best regards, Frank On 11/29/2012 1:08 PM, Frank Ding wrote: Hi Dmitry and Chris, Thanks for your comments. With your comments incorporated, I've prepared v2 @ http://cr.openjdk.java.net/~dingxmin/6512101/webrev.02/. Could you please review it again? Best regards, Frank On 11/14/2012 12:12 AM, Chris Hegarty wrote: On 11/11/2012 07:03 PM, Dmitry Samersoff wrote: Frank, Changes look good for me. I admit that I am not an expert in this area, but given the information you provided, and I guess you verified this in your environment, the conversion would appear reasonable. But it might be better to fall back to original behavior if MultiByteToWideChar return error, rather than abort. I agree with Dmitry, fall back would be preferable. Can you make the changes and post an updated webrev. -Chris. -Dmitry On 2012-11-07 13:08, Frank Ding wrote: Hi guys, Could you please take a look at patch below aimed to resolve existing bug http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6512101 ? http://cr.openjdk.java.net/~dingxmin/6512101/webrev.01/ I happen to have a Chinese Win 7 environment. buggy.png is current output of test case described in bug system whereas fixed.png is the output after the my patch is applied. http://cr.openjdk.java.net/~dingxmin/6512101/buggy.png http://cr.openjdk.java.net/~dingxmin/6512101/fixed.png The patch simply converts to wide chars encoded in CP_OEMCP by calling MultiByteToWideChar. We have confirmed a guy from Microsoft who said that BEGIN QUOTE I'm not sure how common it is to call the Java code that results in calling the GetIfTable API but I would guess it does not happen that often. Additionally, if it's rare that the adapter contains the accented characters, it would definitely be quite easy to miss in testing. I have not found any documentation about the encoding of the bDescr string unfortunately. I did, however, debug through the API and located the place where it is generated. It is getting converted from a UTF-16 string to a single-byte string using a conversion like this: WideCharToMultiByte( CP_OEMCP, WC_NO_BEST_FIT_CHARS, source string, -1, IfRow-bDescr, size, NULL, NULL); I have checked the source for Windows Vista, 2008, Windows 7, and Windows 2008 R2. It is using CP_OEMCP in all of them. So using the reverse conversion in your code using CP_OEMCP should be safe. Alternatively, you can use the GetIfTable2 function (http://msdn.microsoft.com/en-us/library/windows/desktop/aa365945^(v=vs.85).aspx ) which returns the same information in the original UTF-16 encoding. END QUOTE The link below may be helpful to the second param of WideCharToMultiByte. http://blogs.msdn.com/b/oldnewthing/archive/2012/05/04/10300670.aspx You comments are appreciated. Best regards, Frank -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * Give Rabbit time, and he'll always get the answer
Re: Code review: 8003948 (jdk8)
Michael,
150 It might be better to use regionMatches here
162 Is it intentional to always don't copy keys[0]/value[0] ?
Actually I would prefer to have this code better readable,
something like -
157 if ( ! condition ){
keys[j]=keys[j];
value[j]=value[j];
++j
}
-Dmitry
On 2012-12-10 18:30, Michael McMahon wrote:
Could I get the following change reviewed please?
http://cr.openjdk.java.net/~michaelm/8003948/webrev.1/
We need to filter out extraneous authentication headers when doing
ntlm authentication with MS web servers and proxies.
Thanks
Michael
--
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* Give Rabbit time, and he'll always get the answer
Re: reviews for 7200720 and 8003948 [7u-dev]
Michael, On 2012-12-10 23:35, Michael McMahon wrote: Could I get the following webrevs reviewed please? They are identical changes (except for one small change suggested by Dmitry) to what was done in 8 for the same issues http://cr.openjdk.java.net/~michaelm/7200720.7u-dev/webrev.1/ I see a comment // MMM 7200720 ?? at ll: 202 of NTLMAuthentication.java - Does it make sense to change it to something more verbose? Otherwise looks good for me. http://cr.openjdk.java.net/~michaelm/8003948.7u-dev/webrev.1/ Looks good for me. -Dmitry -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * Give Rabbit time, and he'll always get the answer
Re: RFR 8004675: Inet6Address.getHostAddress should use string scope identifier where available
Chris, Looks good for me. PS: Inet6Address.java: It's not necessary to remove explicit initializations - compiler do it perfectly for you. -Dmitry On 2012-12-10 20:01, Chris Hegarty wrote: Inet6Address.getHostAddress() is specified to return the IP address string in textual presentation, followed by a '%' character and the scope identifier. This scope identifier can be either a numeric value or a string, depending on how the instance was created (if it was created with a scoped interface). This change proposes to remove the boolean field, 'scope_ifname_set', since it is not always correctly set when the instance contains a valid scoped interface. For example, when iterating over the NetworkInterface's on the system. 'scope_ifname_set' was never accessed from native code, so it can simply be removed. http://cr.openjdk.java.net/~chegar/8004675/webrev.00/webrev/ -Chris. -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * Give Rabbit time, and he'll always get the answer
Re: Request for code review 6512101: Incorrect encoding in NetworkInterface.getDisplayName()
Frank, Looks good for me. May be Can't get WIDE string should be changed to something more verbose. -Dmitry On 2012-12-10 11:40, Frank Ding wrote: Hi Dmitry and Chris, Could you please review the second revision again? Thanks and Best regards, Frank On 11/29/2012 1:08 PM, Frank Ding wrote: Hi Dmitry and Chris, Thanks for your comments. With your comments incorporated, I've prepared v2 @ http://cr.openjdk.java.net/~dingxmin/6512101/webrev.02/. Could you please review it again? Best regards, Frank On 11/14/2012 12:12 AM, Chris Hegarty wrote: On 11/11/2012 07:03 PM, Dmitry Samersoff wrote: Frank, Changes look good for me. I admit that I am not an expert in this area, but given the information you provided, and I guess you verified this in your environment, the conversion would appear reasonable. But it might be better to fall back to original behavior if MultiByteToWideChar return error, rather than abort. I agree with Dmitry, fall back would be preferable. Can you make the changes and post an updated webrev. -Chris. -Dmitry On 2012-11-07 13:08, Frank Ding wrote: Hi guys, Could you please take a look at patch below aimed to resolve existing bug http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6512101 ? http://cr.openjdk.java.net/~dingxmin/6512101/webrev.01/ I happen to have a Chinese Win 7 environment. buggy.png is current output of test case described in bug system whereas fixed.png is the output after the my patch is applied. http://cr.openjdk.java.net/~dingxmin/6512101/buggy.png http://cr.openjdk.java.net/~dingxmin/6512101/fixed.png The patch simply converts to wide chars encoded in CP_OEMCP by calling MultiByteToWideChar. We have confirmed a guy from Microsoft who said that BEGIN QUOTE I'm not sure how common it is to call the Java code that results in calling the GetIfTable API but I would guess it does not happen that often. Additionally, if it's rare that the adapter contains the accented characters, it would definitely be quite easy to miss in testing. I have not found any documentation about the encoding of the bDescr string unfortunately. I did, however, debug through the API and located the place where it is generated. It is getting converted from a UTF-16 string to a single-byte string using a conversion like this: WideCharToMultiByte( CP_OEMCP, WC_NO_BEST_FIT_CHARS, source string, -1, IfRow-bDescr, size, NULL, NULL); I have checked the source for Windows Vista, 2008, Windows 7, and Windows 2008 R2. It is using CP_OEMCP in all of them. So using the reverse conversion in your code using CP_OEMCP should be safe. Alternatively, you can use the GetIfTable2 function (http://msdn.microsoft.com/en-us/library/windows/desktop/aa365945^(v=vs.85).aspx ) which returns the same information in the original UTF-16 encoding. END QUOTE The link below may be helpful to the second param of WideCharToMultiByte. http://blogs.msdn.com/b/oldnewthing/archive/2012/05/04/10300670.aspx You comments are appreciated. Best regards, Frank -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * Give Rabbit time, and he'll always get the answer
Re: Code review: 7200720: crash in net.dll during NTLM authentication
Michael, NTLMAuthentication.java: 1. Copyright year should be either fixed or not changed 2. I think it's better to remove bug id from comments and replace comment like // MMM 7200720 ?? to something better. NTLMAuthSequence.c 1. Copyright ... Otherwise looks good for me. -Dmitry On 2012-11-28 17:47, Michael McMahon wrote: On 23/11/12 16:16, Alan Bateman wrote: On 23/11/2012 15:59, Michael McMahon wrote: Could I get the following change reviewed please? There was a crash caused by accessing freed memory when authentication was repeated in the same HttpURLConnection instance/ http://cr.openjdk.java.net/~michaelm/7200720/webrev.1/ I haven't reviewed the changes but I notice this changes the build slightly so I just wonder if you've checked the new build. -Alan just checked the new build on Linux and it's fine. There are a couple of other fixes that will follow this one related to this problem. But, I'd like to go ahead and push this one first if possible - Michael -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * Give Rabbit time, and he'll always get the answer
Re: RFR 8003833: Spurious NPE from Socket.getIn/OutputStream
Looks good for me. -Dmitry On 2012-11-27 15:44, Chris Hegarty wrote: This is a longstanding bug in the Socket code that was only noticed recently as a result of some test changes that Daniel pushed in the nio area. There is a very small window in AbstractPlainSocketImpl.getIn/OutputStream where isClosedOrPending() grabs the fdLock to check if the socket is closed, or not, and the construction of the in/output stream, where another thread may asynchronously close the socket. http://cr.openjdk.java.net/~chegar/8003833/webrev.00/webrev/ -Chris. -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * Give Rabbit time, and he'll always get the answer
Re: getportbyname in Java?
John, The goal of getportbyname() is transfer responsibility of finding right ports for running service from software developer to system administrator, and especially useful if we have one /etc/services for whole network (e.g. distributed by NIS) e.g instead of constantly fixing port numbers in jdk tests we can a) require QA to have a name test_program_port defined on all machines b) Use getportbyname() to get appropriate port. or if we have kerberos running on non-standard port, in ideal world we can just change /etc/services:kerberos entry out from 88 instead of fixing zillion client configs. So I'm surprised we don't have this API in java. -Dmitry On 2012-11-14 07:08, John Zavgren wrote: Max: I've never seen a procedure that binds a protocol to a port number. (The http protocol you mention usually uses TCP port number 80, but the port choice for this protocol is up to the discretion of the programmer. The file /etc/services binds names to numbers so that tools like tcpdump and netstat can convert port numbers to names and that can be useful sometimes. But, you can violate these conventions without any consequences. If you use TCP port number 79 to carry http traffic, my netstat program will think it's the finger protocol. You can event set the port number that an http server uses for receiving connections if you want. It's completely legal to run a WWW server on, say, TCP port 666.) On the other hand, there are procedures for getting an IP protocol by name... they will convert the character string UDP into a structure called a protoent i.e., getprotobyname(UDP); The protoent structure in this example will contain the number 17 in host byte order as the member p_proto. But, that's a different kind of protocol than what you are considering. The bindings between IP protocol names and numbers are written in stone, because every OS (windows, Mac, Linux, etc.) needs to know which protocol handler to invoke whenever an IP datagram arrives. If this wasn't true then interoperability would suffer. Maybe this is why getprotobyname() exists in the C runtime libraries? I've never used it. And it doesn't seem necessary. Have you considered using an Enum? That could bind the string http to the port number 80. John - Original Message - From: kurchi.subhra.ha...@oracle.com To: weijun.w...@oracle.com Cc: net-dev@openjdk.java.net Sent: Tuesday, November 13, 2012 9:16:28 PM GMT -05:00 US/Canada Eastern Subject: Re: getportbyname in Java? I don't think so... Thanks, - Kurchi On 13.11.2012 16:40, Weijun Wang wrote: Is there a Java API I can translate http to 80? Thanks Max -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * Give Rabbit time, and he'll always get the answer
Re: Request for code review 6512101: Incorrect encoding in NetworkInterface.getDisplayName()
Frank, Changes look good for me. But it might be better to fall back to original behavior if MultiByteToWideChar return error, rather than abort. -Dmitry On 2012-11-07 13:08, Frank Ding wrote: Hi guys, Could you please take a look at patch below aimed to resolve existing bug http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6512101 ? http://cr.openjdk.java.net/~dingxmin/6512101/webrev.01/ I happen to have a Chinese Win 7 environment. buggy.png is current output of test case described in bug system whereas fixed.png is the output after the my patch is applied. http://cr.openjdk.java.net/~dingxmin/6512101/buggy.png http://cr.openjdk.java.net/~dingxmin/6512101/fixed.png The patch simply converts to wide chars encoded in CP_OEMCP by calling MultiByteToWideChar. We have confirmed a guy from Microsoft who said that BEGIN QUOTE I'm not sure how common it is to call the Java code that results in calling the GetIfTable API but I would guess it does not happen that often. Additionally, if it's rare that the adapter contains the accented characters, it would definitely be quite easy to miss in testing. I have not found any documentation about the encoding of the bDescr string unfortunately. I did, however, debug through the API and located the place where it is generated. It is getting converted from a UTF-16 string to a single-byte string using a conversion like this: WideCharToMultiByte( CP_OEMCP, WC_NO_BEST_FIT_CHARS, source string, -1, IfRow-bDescr, size, NULL, NULL); I have checked the source for Windows Vista, 2008, Windows 7, and Windows 2008 R2. It is using CP_OEMCP in all of them. So using the reverse conversion in your code using CP_OEMCP should be safe. Alternatively, you can use the GetIfTable2 function (http://msdn.microsoft.com/en-us/library/windows/desktop/aa365945^(v=vs.85).aspx ) which returns the same information in the original UTF-16 encoding. END QUOTE The link below may be helpful to the second param of WideCharToMultiByte. http://blogs.msdn.com/b/oldnewthing/archive/2012/05/04/10300670.aspx You comments are appreciated. Best regards, Frank -- Dmitry Samersoff Saint Petersburg, Russia, http://devnull.samersoff.net * There will come soft rains ...
Re: RFR 8002297: sun/net/www/protocol/http/StackTraceTest.java fails intermittently
Chris, If you need definitely refusing port the best option is use port 0 reserved by IANA for this purpose. Other option is use port 1, it reserved for tcp multiplexor service and nowdays is always closed for obvious security reason. -Dmitry On 2012-11-06 15:08, Chris Hegarty wrote: Trivial test issue where the test will fail if run on a machine that has a process listening on port 8080 ( test expects this port to refuse the connection ). The solution is to simply open and close a listening socket and use its ephemeral port. Should be good enough. http://cr.openjdk.java.net/~chegar/8002297/webrev.00/webrev/ -Chris -- Dmitry Samersoff Java development team, SPB04 * There will come soft rains ...
Re: RFR 8002297: sun/net/www/protocol/http/StackTraceTest.java fails intermittently
On 2012-11-06 15:41, Chris Hegarty wrote: On 11/06/2012 11:30 AM, Dmitry Samersoff wrote: Chris, If you need definitely refusing port the best option is use port 0 reserved by IANA for this purpose. Do you have a link specifying this? I don't see this behavior on my Solaris box. http://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xml 0 marked as reserved. -Dmitry -Chris. Other option is use port 1, it reserved for tcp multiplexor service and nowdays is always closed for obvious security reason. -Dmitry On 2012-11-06 15:08, Chris Hegarty wrote: Trivial test issue where the test will fail if run on a machine that has a process listening on port 8080 ( test expects this port to refuse the connection ). The solution is to simply open and close a listening socket and use its ephemeral port. Should be good enough. http://cr.openjdk.java.net/~chegar/8002297/webrev.00/webrev/ -Chris -- Dmitry Samersoff Java development team, SPB04 * There will come soft rains ...
Re: RFR 8000203: file descriptor leak, src/solaris/native/java/net/net_util_md.c ... AND a potential realloc()-related memory leak.
Kurchi. On 2012-10-24 21:31, Kurchi Hazra wrote: Just for the sake of understanding the fix better, loRoutesTemp will point to 0 if the realloc() request fails, and we still need a reference to the older allocated memory (loRoutes in this case) in order to free it. Hence the need for a temporary variable here? Correct. realloc doesn't modify original pointer in case of file. -Dmitry - Kurchi On 24.10.2012 06:27, Chris Hegarty wrote: Looks good to me. Thanks for going the extra mile here. -Chris. On 24/10/2012 14:16, John Zavgren wrote: Greetings: I'm requesting a review of a software change that fixes a file descriptor leak AND a potential memory leak that involves memory reallocation (realloc()). The webrev image is in the following location: http://cr.openjdk.java.net/~chegar/8000203/webrev.01/ Thanks! John Zavgren john.zavg...@oracle.com -- Dmitry Samersoff Java Hotspot development team, SPB04 * There will come soft rains ...
Re: Change to: ./src/solaris/native/java/net/PlainDatagramSocketImpl.c
John, Looks good for me. -Dmitry On 2012-10-19 00:59, John Zavgren wrote: Greetings: I'm proposing the following change: http://cr.openjdk.java.net/~khazra/john/8000206/webrev.00/ because it simplifies the code by eliminating an unnecessary union data structure. This change eliminates a false positive from our static code analyzer: parfait. This modification doesn't change any of the externally-visible behavior in the procedure. I look forward to your comments and suggestions. Thanks! John Zavgren john.zavg...@oracle.com -- Dmitry Samersoff Java Hotspot development team, SPB04 * There will come soft rains ...
hg: jdk8/tl/jdk: 6809322: javax.management.timer.Timer does not fire all notifications
Changeset: b2d8a99a049e Author:dsamersoff Date: 2012-10-17 18:34 +0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/b2d8a99a049e 6809322: javax.management.timer.Timer does not fire all notifications Summary: Some notifications get dropped due to ConcurrentModificationException thrown in Timer.notifyAlarmClock() method. Reviewed-by: dholmes, rbackman Contributed-by: jaroslav.bacho...@oracle.com ! src/share/classes/javax/management/timer/Timer.java + test/javax/management/timer/MissingNotificationTest.java
Re: URL guarantees that the ftp protocol handler is present, worth re-considering this?
Alan, Looks ok for me. -Dmitry On 2012-10-15 20:14, Alan Bateman wrote: java.net.URL guarantees that 5 protocol handlers are present: http, https, ftp, file and jar. ftp a legacy protocol and I'm wondering whether it's time to consider removing it from the list. I'm not suggesting we don't continue to include it, rather just removing the guarantee that it is always present. The motivation for bringing this up is modules and compact profiles where it might be desirable to not include the ftp protocol handler for footprint reasons. -Alan -- Dmitry Samersoff Java Hotspot development team, SPB04 * There will come soft rains ...
hg: jdk8/tl/jdk: 7186723: TEST_BUG: Race condition in sun/management/jmxremote/startstop/JMXStartStopTest.sh
Changeset: 0c1c4b185451 Author:dsamersoff Date: 2012-09-29 15:44 +0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/0c1c4b185451 7186723: TEST_BUG: Race condition in sun/management/jmxremote/startstop/JMXStartStopTest.sh Summary: Make test self-terminating and independent to cygwin/mks kill behaviour Reviewed-by: sspitsyn, alanb ! test/ProblemList.txt ! test/sun/management/jmxremote/startstop/JMXStartStopDoSomething.java ! test/sun/management/jmxremote/startstop/JMXStartStopTest.java ! test/sun/management/jmxremote/startstop/JMXStartStopTest.sh ! test/sun/management/jmxremote/startstop/REMOTE_TESTING.txt
Re: Two Review requests
John,
File Descriptor Leak:
http://cr.openjdk.java.net/~chegar/8000203/webrev.00/
(Jira bug ID number: 8000203)
(*it's not to your changes but as far as you touch this code *)
607 else is not needed here
609 realloc should not touch original pointer in case of fail,
so this code leads to hidden memmory leak
its better to do:
newLoRoutes = realloc(
if (newLoRoutes == NULL){
// What you do here depends to whether you
// plan to keep incomplete table or not.
free(loRoutes);
break;
}
-Dmitry
On 2012-09-28 18:11, John Zavgren wrote:
Greetings:
I just posted the webrev images for two networking code bugs:
File Descriptor Leak:
http://cr.openjdk.java.net/~chegar/8000203/webrev.00/
(Jira bug ID number: 8000203)
Uninitialized memory:
http://cr.openjdk.java.net/~chegar/8000206/webrev.00/
(Jira bug ID: 8000206)
This change doesn't actually fix a bug... the original code initialized
optlen before it was referenced, however, parfait (static code analysis)
believes optlen MAY be used before initialization. I added the assignment
statement to spoof parfait, and it no longer flags a bug. I assume it's
better to put minor harmless tweaks in our code than to add state information
to parfait, that would cause it to ignore certain situations. That option
seems complicated and dangerous.
Thanks!
John Zavgren
john.zavg...@oracle.com
--
Dmitry Samersoff
Java Hotspot development team, SPB04
* There will come soft rains ...
Re: Indenting code?
Paul, On 2012-09-17 17:52, Paul Sandoz wrote: The solution to that is to re-format all JDK source code using a tool and commit independent of any fixes, i bet the NBs formatter could be extracted for command line operation. Netbeans formatter is OK to do simple job why editing, but it couldn't be used as a standalone tool. There are plenty of separate tools, so someone have to invest time to evaluate some of these tools, choose one and write config that mimic netbeans settings. FWIW i really don't care about what the style would be as long as it is not equivalent to a Huggy Bear sofa so garish as to require one to wear peril sensitive sunglasses. Personally, I also don't think that the coding style is important. It's important to have the style common one over all code - which one doesn't really matter. -Dmitry -- Dmitry Samersoff Java Hotspot development team, SPB04 * There will come soft rains ...
Re: Proposed changes for Bug 7193520
John, linux has both ip_mreq and ip_mreqn structs, but after better consideration, I think it's better to *leave this code as is* because ip_mreq is obsoleted and general direction should be to use ip_mreqn for all OS that have it but these changes clearly out of scope of this CR, -Dmitry On 2012-09-15 00:06, John Zavgren wrote: I agree with all the changes you recommend except the last one... see below. - Original Message - From: dmitry.samers...@oracle.com To: john.zavg...@oracle.com Cc: net-dev@openjdk.java.net Sent: Friday, September 14, 2012 3:08:57 PM GMT -05:00 US/Canada Eastern Subject: Re: Proposed changes for Bug 7193520 John, Changes look good for me. Few nits below. PlainDatagramSocketImpl.c 318 brackets is not necessary anymore 1644 whole #ifdef could be removed struct ip_mreqn mreqn; is not necessary anymore, 2283 the same 2294 #ifdef is not necessary anymore --- This is the original code near line number 2294 #ifdef __linux__ mname.imr_address.s_addr = (isOldKernel ? mreqn.imr_address.s_addr : in.s_addr); #else mname.imr_interface.s_addr = in.s_addr; #endif --- When Linux is the OS, the structure field name to be set is imr_address, whereas when other OSes are used, the field name is: imr_interface. Am I understanding your suggestion correctly? Thanks! John -Dmitry On 2012-09-14 22:22, John Zavgren wrote: Greetings: This bug (7193520) was filed because there are obsolete checks in the openJDK native code that implements datagram sockets, e.g., src/solaris/native/java/net/PlainDatagramSocketImpl.c and it's Java counterpart: src/share/classes/java/net/AbstractPlainDatagramSocketImpl.java The native code (PlainDatagramSocketImpl.c and friends) runs real time checks for the Linux kernel version number. If the most significant two fields of the version number is 2.2 on the host platform, then these checks cause the socket to be created, used, and managed differently than if the Linux kernel version were newer. (These behavior changes were necessary because Linux kernel 2.2.X IP networking was was implemented differently and lacked features of the newer kernels.) However, the run time logic isn't actually needed anymore because openJDK doesn't support Linux kernel 2.2.X, and consequently one cannot run openJDK on these older OSes. The run time checks are never used. The proposed changes to the code (http://cr.openjdk.java.net/~chegar/7193520/webrev.00/) eliminate distracting dead wood, and it makes it run (slightly) faster, because the run time checks are eliminated. Thanks! and RSVP John Zavgren john.zavg...@oracle.com -- Dmitry Samersoff Java Hotspot development team, SPB04 * There will come soft rains ...
Re: Indenting code?
Jim, IMHO, It's really boring task to format big peace of code manually. The worst scenario I could imagine - autoformat code by netbeans and then manually adjust it to some coding standard. Is it possible to: 1. Rich some common point between netbeans and old sun coding standard. 2. Provide a config for one of available opensource beautifiers (jalopy, uncrustify etc.) for people who don't use netbeans. 3. Write results as coding standard. -Dmitry On 2012-09-14 21:23, Jim Gish wrote: While it is true that NB and Eclipse and other IDEs offer auto formatting and that will suit some us, I also no that there are some amongst us who still use emacs and vi and possibly other non-IDE editors. The first thing to agree on is what standard are we coding to. I had assumed it was the old Sun Java coding standards ( http://www.oracle.com/technetwork/java/codeconv-138413.html) Is that the case? If not, I suggest that we /don't /open this up to a full-fledged discussion of what the standard should be. I've been involved in far too many such religious debates over the years that end up reminding me of the famous Belushi-esque food fight scene from Animal House. Instead, if any question on any one individual point comes up, we look at the predominate approach in the existing code and use that. As Alan points out, local consistency is important to maintain. In the unlikely event that an entire piece of code is rewritten, then it's ok to bring it up to the current standard, otherwise don't mess with it. In other words, there are more important things to consider than whether any one piece of code meets the standard. Although that would be ideal, we do have to consider the consequences of major formatting changes, since those will impact the ease of interpreting diffs, and far more significant, ability to manage merging. If we agree that the old Sun Java coding standards are what we /are mostly/ using, then we can identify formatting templates for the major IDEs, and other tools as needed. Jim Also, this is broader than net-dev, so I'm moving the discussion to disc...@openjdk.java.net. Please respond there. On 09/14/2012 12:27 PM, Chris Hegarty wrote: On 14/09/12 12:20, Alan Bateman wrote: On 14/09/2012 01:21, Brad Wetmore wrote: Netbean's automatic formatting does a pretty good job with new code. However, I think the general advice is to not change existing code just because. When you're dealing with multiple release families, it makes the merges much more difficult. Brad One think that Paul Sandoz suggested recently is that we should have a NB template that folks can use to avoid some discussions/debates on styles. It would be great for someone to run with that, the hard part is of course that it will be impossible to get agreement. Personally I find NB's defaults okay but there are several cases where its indenting is horrible. I did play with NB somewhat trying to get it follow, exactly, the preferred style in some areas of the JDK code. I was able to get it close, or at least better than the default, but I don't believe it is possible to get it to do exactly what we want. -Chris. Anyway, the main advice I think is to keep things locally consistent where possible. Also major refactoring or formatting in a bug fix is a royal pain for reviewers. -Alan -- Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 Oracle Java Platform Group | Core Libraries Team 35 Network Drive Burlington, MA 01803 jim.g...@oracle.com -- Dmitry Samersoff Java Hotspot development team, SPB04 * There will come soft rains ...
Re: Proposed changes for Bug 7193520
John, Changes look good for me. Few nits below. PlainDatagramSocketImpl.c 318 brackets is not necessary anymore 1644 whole #ifdef could be removed struct ip_mreqn mreqn; is not necessary anymore, 2283 the same 2294 #ifdef is not necessary anymore -Dmitry On 2012-09-14 22:22, John Zavgren wrote: Greetings: This bug (7193520) was filed because there are obsolete checks in the openJDK native code that implements datagram sockets, e.g., src/solaris/native/java/net/PlainDatagramSocketImpl.c and it's Java counterpart: src/share/classes/java/net/AbstractPlainDatagramSocketImpl.java The native code (PlainDatagramSocketImpl.c and friends) runs real time checks for the Linux kernel version number. If the most significant two fields of the version number is 2.2 on the host platform, then these checks cause the socket to be created, used, and managed differently than if the Linux kernel version were newer. (These behavior changes were necessary because Linux kernel 2.2.X IP networking was was implemented differently and lacked features of the newer kernels.) However, the run time logic isn't actually needed anymore because openJDK doesn't support Linux kernel 2.2.X, and consequently one cannot run openJDK on these older OSes. The run time checks are never used. The proposed changes to the code (http://cr.openjdk.java.net/~chegar/7193520/webrev.00/) eliminate distracting dead wood, and it makes it run (slightly) faster, because the run time checks are eliminated. Thanks! and RSVP John Zavgren john.zavg...@oracle.com -- Dmitry Samersoff Java Hotspot development team, SPB04 * There will come soft rains ...
Re: Suggestion of combining some macros of processing solaris, macosx with other UNIX
Chris, Sorry for being later at the party. 1. Could you explain why you are calling both mcast_set_if_by_addr_v4 mcast_set_if_by_addr_v6 for Linux and only mcast_set_if_by_addr_v6 for other OS-es. 2. If it's really necessary did you consider of writing special variant of mcast_set_if_by_addr_v6 combined both functionality for linux to reduce number of JNI (FindClass) calls and macros inside code ? -Dmitry On 2012-08-16 13:21, Frank Ding wrote: On 8/14/2012 10:42 PM, Chris Hegarty wrote: On 14/08/12 13:11, Alan Bateman wrote: On 14/08/2012 04:11, Frank Ding wrote: On 8/7/2012 1:46 PM, Frank Ding wrote: : Could anybody take a look at my patch below and make comment? http://cr.openjdk.java.net/~youdwei/ojdk-533/webrev.00/ Thanks Best regards, Frank Hi all, Is there anybody who is interested in the patch and who can take a look and comment? It looks okay to me but I don't have time at the moment to sponsor it. Can you confirm that you've run the tests with this change? I filed 7191275: Cleanup OS specific vinblocks in PlainDatagramSocketImpl.c to support more unix-like platforms,for this issue. I can sponsor this patch and help get it in. Can answer Alan's question about testing? And confirm that it builds on all platforms? Thanks, -Chris. -Alan Hi Chris and Alan, Thank you for taking time to help this issue. I have built using latest openjdk 8 repo on Windows 64 and Linux 32/64. Since it's a macro change in path src/solaris, I only did jtreg tests for Linux 32 and 64 build. The jtreg tests I ran are restricted to package java/net. Please let me know if you need me to do more tests or on more platforms (such as Solaris). Best regards, Frank -- Dmitry Samersoff Java Hotspot development team, SPB04 * There will come soft rains ...
Re: Suggestion of combining some macros of processing solaris, macosx with other UNIX
Chris, Yes, I'm asking more general question - it's why I address it to you but Frank. IMHO, logic cleanup would take almost the same time as just reformatting macros (in all cases testing is required) and create almost the same disturbance for people porting openjdk to other platforms. So (on my opinion) it's better to do it once. Also do we really supports linux 2.2 kernel in jdk8? I think not, and all related macros should be removed within a scope of any bug touching the file. -Dmitry On 2012-08-16 16:23, Chris Hegarty wrote: You already know this, but to be clear, Frank's changes are simply changing the definitions so that platforms like AIX will use the same implementation as Solaris, Mac, etc. There will be no behavior changes from this. You're just asking general questions, right? On 16/08/2012 12:25, Dmitry Samersoff wrote: Chris, Sorry for being later at the party. 1. Could you explain why you are calling both mcast_set_if_by_addr_v4 mcast_set_if_by_addr_v6 for Linux and only mcast_set_if_by_addr_v6 for other OS-es. There is a lot of history here and many bugs over the years. I'll try to summarize. It was found that the dual stack on Linux requires that both the IPv4 and IPv6 socket options (IP_MULTICAST_IF IPV6_MULTICAST_IF) are required to set the outgoing interface for sending multicast packets. Similarly for other sockets options. This was introduced under CR 4742177 [1]. From 4742177: So with regard to ipv4 and ipv6 socket option, Solaris behaves this way :- - ipv4 options can be set for ipv4 socket fd, and ipv6 options for ipv6 socket fd. Otherwise, an error will occur. - effective options are those belong to the same family of socket fd. Meanwhile, Linux behaves this way :- - both ipv4 options and ipv6 options can be set for a socket fd. - effective options are those belong to the same family of destination address. 2. If it's really necessary did you consider of writing special variant of mcast_set_if_by_addr_v6 combined both functionality for linux to reduce number of JNI (FindClass) calls and macros inside code ? Unfortunately, this behavior is necessary. I agree, the code is not all that pretty, and there is a lot that could be done to clean it up, but I would like to keep this issue confined to Franks's particular problem. -Chris. [1] http://bugs.sun.com/view_bug.do?bug_id=4742177 -Dmitry On 2012-08-16 13:21, Frank Ding wrote: On 8/14/2012 10:42 PM, Chris Hegarty wrote: On 14/08/12 13:11, Alan Bateman wrote: On 14/08/2012 04:11, Frank Ding wrote: On 8/7/2012 1:46 PM, Frank Ding wrote: : Could anybody take a look at my patch below and make comment? http://cr.openjdk.java.net/~youdwei/ojdk-533/webrev.00/ Thanks Best regards, Frank Hi all, Is there anybody who is interested in the patch and who can take a look and comment? It looks okay to me but I don't have time at the moment to sponsor it. Can you confirm that you've run the tests with this change? I filed 7191275: Cleanup OS specific vinblocks in PlainDatagramSocketImpl.c to support more unix-like platforms,for this issue. I can sponsor this patch and help get it in. Can answer Alan's question about testing? And confirm that it builds on all platforms? Thanks, -Chris. -Alan Hi Chris and Alan, Thank you for taking time to help this issue. I have built using latest openjdk 8 repo on Windows 64 and Linux 32/64. Since it's a macro change in path src/solaris, I only did jtreg tests for Linux 32 and 64 build. The jtreg tests I ran are restricted to package java/net. Please let me know if you need me to do more tests or on more platforms (such as Solaris). Best regards, Frank -- Dmitry Samersoff Java Hotspot development team, SPB04 * There will come soft rains ...
Re: Code Review 6972374: NetworkInterface.getNetworkInterfaces throws java.net.SocketException on Solaris zone
Chris, Looks good for me. -Dmitry On 2010-07-28 12:25, Chris Hegarty wrote: Dmitry, Alan, The Solaris version of getFlags sets an Exception if the ioctl fails. When used in addif getFlags will fail when access to the virtual interface's parent is forbidden, i.e. in a zone. addif is called when iterating over interfaces in enumIPvXInterfaces, if an exception occurs it simply cleans up and returns, propagating the exception. getFalgs should not set an exception. All other calls to it check the return value and set an exception if appropriate. Webrev: http://cr.openjdk.java.net/~chegar/6972374/webrev.00/webrev/ Thanks, -Chris. -- Dmitry Samersoff J2SE Sustaining team, SPB04 * Give Rabbit time and he'll always get the answer ...
Re: 6964714, Please review the fix (XS), round 3
On 2010-07-12 22:30, Alan Bateman wrote: Dmitry Samersoff wrote: Alan, Thank you for the comments. Fixed. http://cr.openjdk.java.net/~dsamersoff/6964714/webrev.04/ -Dmitry The copyright date on the new test says 2008 :-) Copy-paste from a neighbor ;) Fixed? http://cr.openjdk.java.net/~dsamersoff/6964714/webrev.05/ -Dmitry -- Dmitry Samersoff J2SE Sustaining team, SPB04 * Give Rabbit time and he'll always get the answer ...
Re: 6964714, Please review the fix (XS), round 3
Chris, Please review my changes. see: http://cr.openjdk.java.net/~dsamersoff/6964714/webrev.03/ -Dmitry -- Dmitry Samersoff J2SE Sustaining team, SPB04 * Give Rabbit time and he'll always get the answer ...
hg: jdk7/tl/jdk: 6931566: NetworkInterface is not working when interface name is more than 15 characters long
Changeset: 887e525597f8 Author:dsamersoff Date: 2010-06-23 17:25 +0400 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/887e525597f8 6931566: NetworkInterface is not working when interface name is more than 15 characters long Summary: Separate Linux and Solaris code, use lifreq under Solaris Reviewed-by: chegar ! src/solaris/native/java/net/NetworkInterface.c
