Re: RFR 8203369 : Check for both EAGAIN and EWOULDBLOCK error codes

[Ivan Gerasimov]

Hi Wiijun!


On 5/24/18 10:13 PM, Weijun Wang wrote:



On May 25, 2018, at 11:58 AM, Ivan Gerasimov  wrote:


I also wonder whether a smart compiler might not flag code where the errors do 
infact have the same value:

if (errno == 11 || errno == 11) ...


At least gcc -O completely removes the second redundant test, so no observable 
changes is expected on supported platforms.

And it silently compiles without showing any warning, right? Good if yes.

--Max


Yep, all is good.
I've built/tested the patched JDK on all supported platforms with no issues.

And we already have places, where both EAGAIN and EWOULDBLOCK are used 
in one if clause (as I just replied to David):

java.base/unix/native/libnet/SocketInputStream.c, in NET_ReadWithTimeout():
result = NET_NonBlockingRead(fd, bufP, len);
if (result == -1 && ((errno == EAGAIN) || (errno == 
EWOULDBLOCK))) {



So the fix basically proposes to use this approach consistently.


With kind regards,
Ivan

--
With kind regards,
Ivan Gerasimov

Re: RFR 8203369 : Check for both EAGAIN and EWOULDBLOCK error codes

[David Holmes]

On 25/05/2018 3:34 PM, Ivan Gerasimov wrote:

Hi David!


On 5/24/18 9:45 PM, David Holmes wrote:


I looked but did not review - it was just an observation :)



Well, thank you anyway :)



It seems pointless to double up these condition checks everywhere 
just in case there is some platform (do we know of one?) where this 
may be necessary.


That's exactly what man pages suggest: "...a portable application 
should check for both..."


Yes but that's the native code that calls the library methods. That 
doesn't necessarily mean we have to propagate the ambiguity through 
our own native wrappers and/or Java code.


Ah, I didn't immediately understood you're talking about constants in 
UnixConstants.java and LinuxWatchService.java.
This part might probably be skipped (because we know that on Linux the 
constants have the same values), but I thought it's better it add it for 
consistency.


In other parts of the fix we do treat the constants uniformly and 
propagate some non-ambiguous value to Java, like returning 
IOS_UNAVAILABLE in most cases.



And yes, there exist such platforms.

I also wonder whether a smart compiler might not flag code where the 
errors do infact have the same value:


if (errno == 11 || errno == 11) ...

At least gcc -O completely removes the second redundant test, so no 
observable changes is expected on supported platforms.


I'm more worried about a new compiler warning - especially if you 
happened to use them in a switch! - resulting in future build failures.




What compiler do you mean: gcc or javac?


gcc

If gcc, then we already have the same test for both constants in code 
with no issues.
For example, java.base/unix/native/libnet/SocketInputStream.c, in 
NET_ReadWithTimeout():

     result = NET_NonBlockingRead(fd, bufP, len);
     if (result == -1 && ((errno == EAGAIN) || (errno == 
EWOULDBLOCK))) {



If javac, then, I was thinking about it too, but I don't have a good a 
universal solution to propose right now.


javac should treat these symbolically rather than based on actual value, 
so I don't see any issue there. It's the C compiler that sees the raw 
value after preprocessing and so sees "duplicate" clauses.


If one day someone needs to use these (platform dependent by definition) 
constants in switch, one will need to invent something to workaround the 
fact that some constants may have the same values on some platforms.
With respect to EAGAIN and EWOULDBLOCK, it will be caught early enough 
because it will fail during the very first build on any currently 
supported Unix platform.


Ok.

Cheers,
David



With kind regards,
Ivan



Cheers,
David


With kind regards,
Ivan


Cheers,
David

On 25/05/2018 6:57 AM, Ivan Gerasimov wrote:

Hello!

On Unix systems several system calls (including pread, read, readv, 
recvfrom, recvmsg, send, sendfile, sendmsg, sendto) may set errno 
to either EAGAIN or EWOULDBLOCK on the same condition.


On Linux these two constants are the same, but they are not 
required to be the same.


For example, here's an extract from the Linux man page of send():
EAGAIN or EWOULDBLOCK
The  socket  is marked nonblocking and the requested operation 
would block.  POSIX.1-2001 allows either error to be returned for 
this case, and does not require these constants to have the same 
value, so a portable application should check for both possibilities.


We should check for both error codes when appropriate.

Would you please help review the fix?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8203369
WEBREV: http://cr.openjdk.java.net/~igerasim/8203369/00/webrev/

Thanks!

Re: RFR 8203369 : Check for both EAGAIN and EWOULDBLOCK error codes

[Ivan Gerasimov]

Hi David!


On 5/24/18 9:45 PM, David Holmes wrote:


I looked but did not review - it was just an observation :)



Well, thank you anyway :)



It seems pointless to double up these condition checks everywhere 
just in case there is some platform (do we know of one?) where this 
may be necessary.


That's exactly what man pages suggest: "...a portable application 
should check for both..."


Yes but that's the native code that calls the library methods. That 
doesn't necessarily mean we have to propagate the ambiguity through 
our own native wrappers and/or Java code.


Ah, I didn't immediately understood you're talking about constants in 
UnixConstants.java and LinuxWatchService.java.
This part might probably be skipped (because we know that on Linux the 
constants have the same values), but I thought it's better it add it for 
consistency.


In other parts of the fix we do treat the constants uniformly and 
propagate some non-ambiguous value to Java, like returning 
IOS_UNAVAILABLE in most cases.



And yes, there exist such platforms.

I also wonder whether a smart compiler might not flag code where the 
errors do infact have the same value:


if (errno == 11 || errno == 11) ...

At least gcc -O completely removes the second redundant test, so no 
observable changes is expected on supported platforms.


I'm more worried about a new compiler warning - especially if you 
happened to use them in a switch! - resulting in future build failures.




What compiler do you mean: gcc or javac?

If gcc, then we already have the same test for both constants in code 
with no issues.
For example, java.base/unix/native/libnet/SocketInputStream.c, in 
NET_ReadWithTimeout():

result = NET_NonBlockingRead(fd, bufP, len);
if (result == -1 && ((errno == EAGAIN) || (errno == 
EWOULDBLOCK))) {



If javac, then, I was thinking about it too, but I don't have a good a 
universal solution to propose right now.
If one day someone needs to use these (platform dependent by definition) 
constants in switch, one will need to invent something to workaround the 
fact that some constants may have the same values on some platforms.
With respect to EAGAIN and EWOULDBLOCK, it will be caught early enough 
because it will fail during the very first build on any currently 
supported Unix platform.



With kind regards,
Ivan



Cheers,
David


With kind regards,
Ivan


Cheers,
David

On 25/05/2018 6:57 AM, Ivan Gerasimov wrote:

Hello!

On Unix systems several system calls (including pread, read, readv, 
recvfrom, recvmsg, send, sendfile, sendmsg, sendto) may set errno 
to either EAGAIN or EWOULDBLOCK on the same condition.


On Linux these two constants are the same, but they are not 
required to be the same.


For example, here's an extract from the Linux man page of send():
EAGAIN or EWOULDBLOCK
The  socket  is marked nonblocking and the requested operation 
would block.  POSIX.1-2001 allows either error to be returned for 
this case, and does not require these constants to have the same 
value, so a portable application should check for both possibilities.


We should check for both error codes when appropriate.

Would you please help review the fix?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8203369
WEBREV: http://cr.openjdk.java.net/~igerasim/8203369/00/webrev/

Thanks!









--
With kind regards,
Ivan Gerasimov

Re: RFR 8203369 : Check for both EAGAIN and EWOULDBLOCK error codes

[Weijun Wang]

https://stackoverflow.com/questions/27509061/macro-to-avoid-duplicate-case-value

Someone has already used them in a switch expression...

> On May 25, 2018, at 12:45 PM, David Holmes  wrote:
> 
>>> I also wonder whether a smart compiler might not flag code where the errors 
>>> do infact have the same value:
>>> 
>>> if (errno == 11 || errno == 11) ...
>>> 
>> At least gcc -O completely removes the second redundant test, so no 
>> observable changes is expected on supported platforms.
> 
> I'm more worried about a new compiler warning - especially if you happened to 
> use them in a switch! - resulting in future build failures.

Re: RFR 8203369 : Check for both EAGAIN and EWOULDBLOCK error codes

[Weijun Wang]

> On May 25, 2018, at 11:58 AM, Ivan Gerasimov  
> wrote:
> 
>> I also wonder whether a smart compiler might not flag code where the errors 
>> do infact have the same value:
>> 
>> if (errno == 11 || errno == 11) ...
>> 
> At least gcc -O completely removes the second redundant test, so no 
> observable changes is expected on supported platforms.

And it silently compiles without showing any warning, right? Good if yes.

--Max

Re: RFR 8203369 : Check for both EAGAIN and EWOULDBLOCK error codes

[David Holmes]

Hi Ivan,

On 25/05/2018 1:58 PM, Ivan Gerasimov wrote:

Hi David!

Thank you for reviewing the fix!


I looked but did not review - it was just an observation :)



On 5/24/18 7:44 PM, David Holmes wrote:

Hi Ivan,

Just a thought, but just because the actual native function may return 
either code, that doesn't mean our native wrapper can't treat them the 
same and present them to the Java code as one error?


Currently in some places we check for only one of the values (on the 
supported platforms we could have being checking for the other with 
exactly same effect).  In other places we already check for both values, 
so it is proposed to do it consistently with accordance to the 
documentation.


It seems pointless to double up these condition checks everywhere just 
in case there is some platform (do we know of one?) where this may be 
necessary.


That's exactly what man pages suggest: "...a portable application should 
check for both..."


Yes but that's the native code that calls the library methods. That 
doesn't necessarily mean we have to propagate the ambiguity through our 
own native wrappers and/or Java code.



And yes, there exist such platforms.

I also wonder whether a smart compiler might not flag code where the 
errors do infact have the same value:


if (errno == 11 || errno == 11) ...

At least gcc -O completely removes the second redundant test, so no 
observable changes is expected on supported platforms.


I'm more worried about a new compiler warning - especially if you 
happened to use them in a switch! - resulting in future build failures.


Cheers,
David


With kind regards,
Ivan


Cheers,
David

On 25/05/2018 6:57 AM, Ivan Gerasimov wrote:

Hello!

On Unix systems several system calls (including pread, read, readv, 
recvfrom, recvmsg, send, sendfile, sendmsg, sendto) may set errno to 
either EAGAIN or EWOULDBLOCK on the same condition.


On Linux these two constants are the same, but they are not required 
to be the same.


For example, here's an extract from the Linux man page of send():
EAGAIN or EWOULDBLOCK
The  socket  is marked nonblocking and the requested operation would 
block.  POSIX.1-2001 allows either error to be returned for this 
case, and does not require these constants to have the same value, so 
a portable application should check for both possibilities.


We should check for both error codes when appropriate.

Would you please help review the fix?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8203369
WEBREV: http://cr.openjdk.java.net/~igerasim/8203369/00/webrev/

Thanks!

Re: RFR 8203369 : Check for both EAGAIN and EWOULDBLOCK error codes

[Ivan Gerasimov]

Hi David!

Thank you for reviewing the fix!


On 5/24/18 7:44 PM, David Holmes wrote:

Hi Ivan,

Just a thought, but just because the actual native function may return 
either code, that doesn't mean our native wrapper can't treat them the 
same and present them to the Java code as one error?


Currently in some places we check for only one of the values (on the 
supported platforms we could have being checking for the other with 
exactly same effect).  In other places we already check for both values, 
so it is proposed to do it consistently with accordance to the 
documentation.


It seems pointless to double up these condition checks everywhere just 
in case there is some platform (do we know of one?) where this may be 
necessary.


That's exactly what man pages suggest: "...a portable application should 
check for both..."

And yes, there exist such platforms.

I also wonder whether a smart compiler might not flag code where the 
errors do infact have the same value:


if (errno == 11 || errno == 11) ...

At least gcc -O completely removes the second redundant test, so no 
observable changes is expected on supported platforms.


With kind regards,
Ivan


Cheers,
David

On 25/05/2018 6:57 AM, Ivan Gerasimov wrote:

Hello!

On Unix systems several system calls (including pread, read, readv, 
recvfrom, recvmsg, send, sendfile, sendmsg, sendto) may set errno to 
either EAGAIN or EWOULDBLOCK on the same condition.


On Linux these two constants are the same, but they are not required 
to be the same.


For example, here's an extract from the Linux man page of send():
EAGAIN or EWOULDBLOCK
The  socket  is marked nonblocking and the requested operation would 
block.  POSIX.1-2001 allows either error to be returned for this 
case, and does not require these constants to have the same value, so 
a portable application should check for both possibilities.


We should check for both error codes when appropriate.

Would you please help review the fix?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8203369
WEBREV: http://cr.openjdk.java.net/~igerasim/8203369/00/webrev/

Thanks!





--
With kind regards,
Ivan Gerasimov

Re: RFR 8203369 : Check for both EAGAIN and EWOULDBLOCK error codes

[David Holmes]

Hi Ivan,

Just a thought, but just because the actual native function may return 
either code, that doesn't mean our native wrapper can't treat them the 
same and present them to the Java code as one error?


It seems pointless to double up these condition checks everywhere just 
in case there is some platform (do we know of one?) where this may be 
necessary.


I also wonder whether a smart compiler might not flag code where the 
errors do infact have the same value:


if (errno == 11 || errno == 11) ...

Cheers,
David

On 25/05/2018 6:57 AM, Ivan Gerasimov wrote:

Hello!

On Unix systems several system calls (including pread, read, readv, 
recvfrom, recvmsg, send, sendfile, sendmsg, sendto) may set errno to 
either EAGAIN or EWOULDBLOCK on the same condition.


On Linux these two constants are the same, but they are not required to 
be the same.


For example, here's an extract from the Linux man page of send():
EAGAIN or EWOULDBLOCK
The  socket  is marked nonblocking and the requested operation would 
block.  POSIX.1-2001 allows either error to be returned for this case, 
and does not require these constants to have the same value, so a 
portable application should check for both possibilities.


We should check for both error codes when appropriate.

Would you please help review the fix?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8203369
WEBREV: http://cr.openjdk.java.net/~igerasim/8203369/00/webrev/

Thanks!

RFR 8203369 : Check for both EAGAIN and EWOULDBLOCK error codes

[Ivan Gerasimov]

Hello!

On Unix systems several system calls (including pread, read, readv, 
recvfrom, recvmsg, send, sendfile, sendmsg, sendto) may set errno to 
either EAGAIN or EWOULDBLOCK on the same condition.


On Linux these two constants are the same, but they are not required to 
be the same.


For example, here's an extract from the Linux man page of send():
EAGAIN or EWOULDBLOCK
The  socket  is marked nonblocking and the requested operation would 
block.  POSIX.1-2001 allows either error to be returned for this case, 
and does not require these constants to have the same value, so a 
portable application should check for both possibilities.


We should check for both error codes when appropriate.

Would you please help review the fix?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8203369
WEBREV: http://cr.openjdk.java.net/~igerasim/8203369/00/webrev/

Thanks!

--
With kind regards,
Ivan Gerasimov