Ping: RFR(S): 8163181: Further improvements for Unix NetworkInterface native implementation

2016-09-02 Thread Langer, Christoph 
Hi (Mark or Chris?),

as this RFR is outstanding for quite a while and it merely is a minor change 
which only cleans up coding, would you mind to review it that I can get it off 
my list?

The only major thing probably is, that calls to getMacAddress don't need a 
socket anymore and sockets are only opened for platforms where an ioctl is done 
to determine the MacAddress.

As I've said, I've tested it on the Linux/Unix platforms.

Thanks a lot
Christoph

From: Langer, Christoph
Sent: Montag, 22. August 2016 15:30
To: 'net-dev@openjdk.java.net' 
Subject: RE: Ping: RFR(S): 8163181: Further improvements for Unix 
NetworkInterface native implementation

Hi,

I made a little update to my change: 
http://cr.openjdk.java.net/~clanger/webrevs/8163181.2/

Merely comments and a minor AIX specific correction.

Thanks in advance for reviewing
Christoph


From: Langer, Christoph
Sent: Dienstag, 16. August 2016 14:49
To: 'net-dev@openjdk.java.net' 
mailto:net-dev@openjdk.java.net>>
Subject: Ping: RFR(S): 8163181: Further improvements for Unix NetworkInterface 
native implementation

Ping: Can I get a review for this small set of changes please?

Thanks
Christoph

From: Langer, Christoph
Sent: Donnerstag, 4. August 2016 15:04
To: net-dev@openjdk.java.net
Subject: RFR(S): 8163181: Further improvements for Unix NetworkInterface native 
implementation

Hi,

I had made a few more cleanups when I was working on NetworkInterface.c which I 
thought are worth contributing.

Please review: http://cr.openjdk.java.net/~clanger/webrevs/8163181.1/
Bug: https://bugs.openjdk.java.net/browse/JDK-8163181

As always, changes were built and tested on Linux, AIX, Solaris and Mac.

Thanks
Christoph

Re: Ping: RFR(S): 8163181: Further improvements for Unix NetworkInterface native implementation

2016-09-02 Thread Chris Hegarty 
Apologies Christoph, this dropped off my list and I forgot to get back to it.

> On 2 Sep 2016, at 08:22, Langer, Christoph  wrote:
> 
> Hi (Mark or Chris?),
>  
> as this RFR is outstanding for quite a while and it merely is a minor change 
> which only cleans up coding, would you mind to review it that I can get it 
> off my list?
>  
> The only major thing probably is, that calls to getMacAddress don’t need a 
> socket anymore and sockets are only opened for platforms where an ioctl is 
> done to determine the MacAddress.
>  
> As I’ve said, I’ve tested it on the Linux/Unix platforms.
>  
> Thanks a lot
> Christoph
>  
> From: Langer, Christoph 
> Sent: Montag, 22. August 2016 15:30
> To: 'net-dev@openjdk.java.net' 
> Subject: RE: Ping: RFR(S): 8163181: Further improvements for Unix 
> NetworkInterface native implementation
>  
> Hi,
>  
> I made a little update to my change: 
> http://cr.openjdk.java.net/~clanger/webrevs/8163181.2/


Your changes look fine to me. This is a natural cleanup now that we have
moved away from ioctl on some platforms.

I will submit your patch to our internal build and test system, and report
back the results later today.

-Chris.


> Merely comments and a minor AIX specific correction.
>  
> Thanks in advance for reviewing
> Christoph
>  
>  
> From: Langer, Christoph 
> Sent: Dienstag, 16. August 2016 14:49
> To: 'net-dev@openjdk.java.net' 
> Subject: Ping: RFR(S): 8163181: Further improvements for Unix 
> NetworkInterface native implementation
>  
> Ping: Can I get a review for this small set of changes please?
>  
> Thanks
> Christoph
>  
> From: Langer, Christoph 
> Sent: Donnerstag, 4. August 2016 15:04
> To: net-dev@openjdk.java.net
> Subject: RFR(S): 8163181: Further improvements for Unix NetworkInterface 
> native implementation
>  
> Hi,
>  
> I had made a few more cleanups when I was working on NetworkInterface.c which 
> I thought are worth contributing.
>  
> Please review: http://cr.openjdk.java.net/~clanger/webrevs/8163181.1/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8163181
>  
> As always, changes were built and tested on Linux, AIX, Solaris and Mac.
>  
> Thanks
> Christoph

Re: RFR 8075484:SocketInputStream.socketRead0 can hang even with soTimeout set

2016-09-02 Thread Dmitry Samersoff 
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
>>> ). 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(&t, NULL);
 newtime = t.tv_sec * 1000 + t.tv_usec / 1000;
 _timeout -= newtime - prevtime;
 if(_timeout > 0){
 prevtime = newtime;
}
 } else { break; } } } return nread;

 e&oe

 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
>> 
>>
>> 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: Ping: RFR(S): 8163181: Further improvements for Unix NetworkInterface native implementation

2016-09-02 Thread Mark Sheppard 


have had a look through the changes twice, and they look fine ... i'll 
apply the patch and run a regression build to confirm


the moving of int flags on 919 to a conditional block,  I expect to 
cause a strict compile error in my solaris env, so need to check that


regards
Mark

On 02/09/2016 08:22, Langer, Christoph wrote:


Hi (Mark or Chris?),

as this RFR is outstanding for quite a while and it merely is a minor 
change which only cleans up coding, would you mind to review it that I 
can get it off my list?


The only major thing probably is, that calls to getMacAddress don’t 
need a socket anymore and sockets are only opened for platforms where 
an ioctl is done to determine the MacAddress.


As I’ve said, I’ve tested it on the Linux/Unix platforms.

Thanks a lot

Christoph

*From:*Langer, Christoph
*Sent:* Montag, 22. August 2016 15:30
*To:* 'net-dev@openjdk.java.net' 
*Subject:* RE: Ping: RFR(S): 8163181: Further improvements for Unix 
NetworkInterface native implementation


Hi,

I made a little update to my change: 
http://cr.openjdk.java.net/~clanger/webrevs/8163181.2/ 



Merely comments and a minor AIX specific correction.

Thanks in advance for reviewing

Christoph

*From:*Langer, Christoph
*Sent:* Dienstag, 16. August 2016 14:49
*To:* 'net-dev@openjdk.java.net' >
*Subject:* Ping: RFR(S): 8163181: Further improvements for Unix 
NetworkInterface native implementation


Ping: Can I get a review for this small set of changes please?

Thanks

Christoph

*From:*Langer, Christoph
*Sent:* Donnerstag, 4. August 2016 15:04
*To:* net-dev@openjdk.java.net 
*Subject:* RFR(S): 8163181: Further improvements for Unix 
NetworkInterface native implementation


Hi,

I had made a few more cleanups when I was working on 
NetworkInterface.c which I thought are worth contributing.


Please review: http://cr.openjdk.java.net/~clanger/webrevs/8163181.1/ 



Bug: https://bugs.openjdk.java.net/browse/JDK-8163181 



As always, changes were built and tested on Linux, AIX, Solaris and Mac.

Thanks

Christoph

Re: Review request JDK-8165180: Provide a shared secret to access non-public ServerSocket constructor

2016-09-02 Thread Peter Levart 

Hi Many,

Are you sure the implementation class passed to 
JavaNetSocketAccess.newSocketImpl(Class implClass) 
is never going to be loaded by a class loader other than bootstrap 
classloader (the loader of the caller of 
implClass.getDeclaredConstructor()) and that no unprivileged code will 
be on the call stack at that time? Do you need to enclose this 
invocation into doPrivileged() block or do you expect that the caller of 
JavaNetSocketAccess.newSocketImpl() will do that?


Regards, Peter

On 08/31/2016 10:48 PM, Mandy Chung wrote:

This patch introduces JavaNetSocketAccess to allow access to non-public 
ServerSocket constructor that is accessed by some other area as a clean up.

Webrev:
   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8165180/webrev.00/

Mandy

[8u-dev] RFR (XXS) and request for approval: 8165320: Small flaw when integrating 8160174 to JDK8

2016-09-02 Thread Langer, Christoph 
Hi,

can you please review and approve a tiny and straightforward fix for AIX only 
in JDK8.

Bug: https://bugs.openjdk.java.net/browse/JDK-8165320
Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8165320.8udev/

When integrating 8160174, I oversaw 2 minor places where JVM_Socket should be 
used instead of calling the socket API directly for the AIX branch. All other 
operating systems are not affected and the code is correct there.

@Chris: Maybe you can review this straightforward?

Thanks & Best regards
Christoph

Re: Ping: RFR(S): 8163181: Further improvements for Unix NetworkInterface native implementation

2016-09-02 Thread Mark Sheppard 


builds and regression tests appear to be OK with the proposed changes.

regards
Mark

On 02/09/2016 12:41, Mark Sheppard wrote:


have had a look through the changes twice, and they look fine ... i'll 
apply the patch and run a regression build to confirm


the moving of int flags on 919 to a conditional block,  I expect to 
cause a strict compile error in my solaris env, so need to check that


regards
Mark

On 02/09/2016 08:22, Langer, Christoph wrote:


Hi (Mark or Chris?),

as this RFR is outstanding for quite a while and it merely is a minor 
change which only cleans up coding, would you mind to review it that 
I can get it off my list?


The only major thing probably is, that calls to getMacAddress don’t 
need a socket anymore and sockets are only opened for platforms where 
an ioctl is done to determine the MacAddress.


As I’ve said, I’ve tested it on the Linux/Unix platforms.

Thanks a lot

Christoph

*From:*Langer, Christoph
*Sent:* Montag, 22. August 2016 15:30
*To:* 'net-dev@openjdk.java.net' 
*Subject:* RE: Ping: RFR(S): 8163181: Further improvements for Unix 
NetworkInterface native implementation


Hi,

I made a little update to my change: 
http://cr.openjdk.java.net/~clanger/webrevs/8163181.2/ 



Merely comments and a minor AIX specific correction.

Thanks in advance for reviewing

Christoph

*From:*Langer, Christoph
*Sent:* Dienstag, 16. August 2016 14:49
*To:* 'net-dev@openjdk.java.net' >
*Subject:* Ping: RFR(S): 8163181: Further improvements for Unix 
NetworkInterface native implementation


Ping: Can I get a review for this small set of changes please?

Thanks

Christoph

*From:*Langer, Christoph
*Sent:* Donnerstag, 4. August 2016 15:04
*To:* net-dev@openjdk.java.net 
*Subject:* RFR(S): 8163181: Further improvements for Unix 
NetworkInterface native implementation


Hi,

I had made a few more cleanups when I was working on 
NetworkInterface.c which I thought are worth contributing.


Please review: http://cr.openjdk.java.net/~clanger/webrevs/8163181.1/ 



Bug: https://bugs.openjdk.java.net/browse/JDK-8163181 



As always, changes were built and tested on Linux, AIX, Solaris and Mac.

Thanks

Christoph

RE: Ping: RFR(S): 8163181: Further improvements for Unix NetworkInterface native implementation

2016-09-02 Thread Langer, Christoph 
That's great, thanks.

I'll push on Monday.

Best regards
Christoph

From: Mark Sheppard [mailto:mark.shepp...@oracle.com]
Sent: Freitag, 2. September 2016 16:50
To: Langer, Christoph ; net-dev@openjdk.java.net
Subject: Re: Ping: RFR(S): 8163181: Further improvements for Unix 
NetworkInterface native implementation


builds and regression tests appear to be OK with the proposed changes.

regards
Mark
On 02/09/2016 12:41, Mark Sheppard wrote:

have had a look through the changes twice, and they look fine ... i'll apply 
the patch and run a regression build to confirm

the moving of int flags on 919 to a conditional block,  I expect to cause a 
strict compile error in my solaris env, so need to check that

regards
Mark
On 02/09/2016 08:22, Langer, Christoph wrote:
Hi (Mark or Chris?),

as this RFR is outstanding for quite a while and it merely is a minor change 
which only cleans up coding, would you mind to review it that I can get it off 
my list?

The only major thing probably is, that calls to getMacAddress don't need a 
socket anymore and sockets are only opened for platforms where an ioctl is done 
to determine the MacAddress.

As I've said, I've tested it on the Linux/Unix platforms.

Thanks a lot
Christoph

From: Langer, Christoph
Sent: Montag, 22. August 2016 15:30
To: 'net-dev@openjdk.java.net' 

Subject: RE: Ping: RFR(S): 8163181: Further improvements for Unix 
NetworkInterface native implementation

Hi,

I made a little update to my change: 
http://cr.openjdk.java.net/~clanger/webrevs/8163181.2/

Merely comments and a minor AIX specific correction.

Thanks in advance for reviewing
Christoph


From: Langer, Christoph
Sent: Dienstag, 16. August 2016 14:49
To: 'net-dev@openjdk.java.net' 
mailto:net-dev@openjdk.java.net>>
Subject: Ping: RFR(S): 8163181: Further improvements for Unix NetworkInterface 
native implementation

Ping: Can I get a review for this small set of changes please?

Thanks
Christoph

From: Langer, Christoph
Sent: Donnerstag, 4. August 2016 15:04
To: net-dev@openjdk.java.net
Subject: RFR(S): 8163181: Further improvements for Unix NetworkInterface native 
implementation

Hi,

I had made a few more cleanups when I was working on NetworkInterface.c which I 
thought are worth contributing.

Please review: 
http://cr.openjdk.java.net/~clanger/webrevs/8163181.1/
Bug: https://bugs.openjdk.java.net/browse/JDK-8163181

As always, changes were built and tested on Linux, AIX, Solaris and Mac.

Thanks
Christoph

Re: [8u-dev] RFR (XXS) and request for approval: 8165320: Small flaw when integrating 8160174 to JDK8

2016-09-02 Thread Chris Hegarty 

On 02/09/16 15:02, Langer, Christoph wrote:

Hi,



can you please review and approve a tiny and straightforward fix for AIX
only in JDK8.



Bug: https://bugs.openjdk.java.net/browse/JDK-8165320

Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8165320.8udev/


This looks fine. Reviewed.

-Chris.



When integrating 8160174, I oversaw 2 minor places where JVM_Socket
should be used instead of calling the socket API directly for the AIX
branch. All other operating systems are not affected and the code is
correct there.



@Chris: Maybe you can review this straightforward?



Thanks & Best regards

Christoph

Re: Review request JDK-8165180: Provide a shared secret to access non-public ServerSocket constructor

2016-09-02 Thread Mandy Chung 
Constructor::newInstance is a caller-sensitive method that performs the 
security check when the caller is not the same class loader as implClass’s 
class loader or not its ancestor.

In this case the caller class is ServerSocket and its class loader is the 
bootstrap class loader is privileged (an ancestor of any loader).

Mandy

> On Sep 2, 2016, at 6:42 AM, Peter Levart  wrote:
> 
> Hi Many,
> 
> Are you sure the implementation class passed to 
> JavaNetSocketAccess.newSocketImpl(Class implClass) is 
> never going to be loaded by a class loader other than bootstrap classloader 
> (the loader of the caller of implClass.getDeclaredConstructor()) and that no 
> unprivileged code will be on the call stack at that time? Do you need to 
> enclose this invocation into doPrivileged() block or do you expect that the 
> caller of JavaNetSocketAccess.newSocketImpl() will do that?
> 
> Regards, Peter
> 
> On 08/31/2016 10:48 PM, Mandy Chung wrote:
>> This patch introduces JavaNetSocketAccess to allow access to non-public 
>> ServerSocket constructor that is accessed by some other area as a clean up.
>> 
>> Webrev:
>>   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8165180/webrev.00/
>> 
>> Mandy
>