RFR 8203369 : Check for both EAGAIN and EWOULDBLOCK error codes
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
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!
Re: RFR 8203369 : Check for both EAGAIN and EWOULDBLOCK error codes
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
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
> 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
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
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
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
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
On Friday 25 May 2018 11:19 AM, Ivan Gerasimov wrote: 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))) { i wrote this code and that time even i wanted to fix the other native code (check error code for both EAGAIN & EWOULDBLOCK) as you did ,but i did not . So the fix basically proposes to use this approach consistently. we can at least fix the native part of code. Vyom With kind regards, Ivan
Re: RFR 8203369 : Check for both EAGAIN and EWOULDBLOCK error codes
Hi David! 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 fact, as UnixContant.java is built from the template file UnixConstants.java.template, which is processed with cpp preprocessor, javac also sees the raw values. So if someone decides to write something like switch (exc.errno()) { case UnixConstants.EAGAIN: case UnixConstants.EWOULDBLOCK: } then there will be compile time trouble on some platforms and no issues on the others. Fortunately, UnixContant class is package private, so it is quite unlikely to happen. I was even thinking about proposing a language-level enhancement: If a switch statement contains duplicate cases *and* these are combined, then treat them as just one case. Though the use case it too limited to justify the need :) With kind regards, Ivan
Re: RFR 8203369 : Check for both EAGAIN and EWOULDBLOCK error codes
On 5/24/18 10:25 PM, Weijun Wang wrote: https://stackoverflow.com/questions/27509061/macro-to-avoid-duplicate-case-value Someone has already used them in a switch expression... Interesting! I like the very first solution: #if EAGAIN != EWOULDBLOCK case EAGAIN: #endif case EWOULDBLOCK: It is descriptive enough and compiles optimally. Fortunately, we don't have to care about it now, as the fix only adds checks of errno right after calls to system functions and does not store the errno value anywhere to analyze it later. -- With kind regards, Ivan Gerasimov
Re: RFR 8203369 : Check for both EAGAIN and EWOULDBLOCK error codes
On 25/05/2018 4:14 PM, Ivan Gerasimov wrote: Hi David! 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 fact, as UnixContant.java is built from the template file UnixConstants.java.template, which is processed with cpp preprocessor, javac also sees the raw values. So if someone decides to write something like switch (exc.errno()) { case UnixConstants.EAGAIN: case UnixConstants.EWOULDBLOCK: } then there will be compile time trouble on some platforms and no issues on the others. I was thinking javac wouldn't examine the actual value of the fields, but it does for switch :( Oh well. David - Fortunately, UnixContant class is package private, so it is quite unlikely to happen. I was even thinking about proposing a language-level enhancement: If a switch statement contains duplicate cases *and* these are combined, then treat them as just one case. Though the use case it too limited to justify the need :) With kind regards, Ivan
Re: RFR 8203369 : Check for both EAGAIN and EWOULDBLOCK error codes
On 24/05/2018 21:57, 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/ The changes in 01/webrev look okay to me but I'm curious if there are any ports in OpenJDK where the values are different. If they are different then I would assume that many tests will fail. -Alan
Re: RFR 8203369 : Check for both EAGAIN and EWOULDBLOCK error codes
Alan, On 25/05/18 12:27, Alan Bateman wrote: On 24/05/2018 21:57, Ivan Gerasimov wrote: .. WEBREV: http://cr.openjdk.java.net/~igerasim/8203369/00/webrev/ The changes in 01/webrev look okay to me but I'm curious if there are any ports in OpenJDK where the values are different. HPUX 11.31 ( on Itanium IA-64 ) does indeed have EAGAIN and EWOULDBLOCK defined to be different values. This is the main motivation for resolving this issue. /usr/include/sys/errno.h:#defineEAGAIN 11 /* No more processes*/ /usr/include/sys/errno.h:#defineEWOULDBLOCK 246 /* Operation would block */ > If they are different then I would assume that many tests will fail. Yes. -Chris.