Re: RFR: 8259943: FileDescriptor.close0 does not handle EINTR [v2]
On Thu, 21 Jan 2021 18:58:53 GMT, Alan Bateman wrote:
>> Brian Burkhalter has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8259943: Suppress RESTARTABLE for non-AIX.
>
> src/java.base/unix/native/libjava/io_util_md.c line 173:
>
>> 171: result = close(fd);
>> 172: #endif
>> 173: if (result == -1) {
>
> This needs to be `if (result == -1 && errno != EINTR)`
Yes, I already caught and fixed that gaffe.
-
PR: https://git.openjdk.java.net/jdk/pull/2173
Re: RFR: 8259943: FileDescriptor.close0 does not handle EINTR [v3]
On Thu, 21 Jan 2021 19:12:16 GMT, Brian Burkhalter wrote: >> Please review this small change to handle `EINTR` from `close()` in >> `fileDescriptorClose()`. The function `handleGetLength()` is also changed to >> handle `EINTR` from `fstat64()` to match other uses of `fstat64()` in the >> file. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 8259943: Added unintentionally omitted EINTR check. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2173
Re: RFR: 8259943: FileDescriptor.close0 does not handle EINTR [v3]
> Please review this small change to handle `EINTR` from `close()` in > `fileDescriptorClose()`. The function `handleGetLength()` is also changed to > handle `EINTR` from `fstat64()` to match other uses of `fstat64()` in the > file. Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision: 8259943: Added unintentionally omitted EINTR check. - Changes: - all: https://git.openjdk.java.net/jdk/pull/2173/files - new: https://git.openjdk.java.net/jdk/pull/2173/files/d351118a..45a5d188 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2173=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2173=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2173.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2173/head:pull/2173 PR: https://git.openjdk.java.net/jdk/pull/2173
Re: RFR: 8259943: FileDescriptor.close0 does not handle EINTR [v2]
On Thu, 21 Jan 2021 18:56:59 GMT, Brian Burkhalter wrote:
>> Please review this small change to handle `EINTR` from `close()` in
>> `fileDescriptorClose()`. The function `handleGetLength()` is also changed to
>> handle `EINTR` from `fstat64()` to match other uses of `fstat64()` in the
>> file.
>
> Brian Burkhalter has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 8259943: Suppress RESTARTABLE for non-AIX.
src/java.base/unix/native/libjava/io_util_md.c line 173:
> 171: result = close(fd);
> 172: #endif
> 173: if (result == -1) {
This needs to be `if (result == -1 && errno != EINTR)`
-
PR: https://git.openjdk.java.net/jdk/pull/2173
Re: RFR: 8259943: FileDescriptor.close0 does not handle EINTR [v2]
> On Jan 21, 2021, at 11:06 AM, Alan Bateman wrote:
>
> On Thu, 21 Jan 2021 18:56:59 GMT, Brian Burkhalter wrote:
>
>>> Please review this small change to handle `EINTR` from `close()` in
>>> `fileDescriptorClose()`. The function `handleGetLength()` is also changed
>>> to handle `EINTR` from `fstat64()` to match other uses of `fstat64()` in
>>> the file.
>>
>> Brian Burkhalter has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8259943: Suppress RESTARTABLE for non-AIX.
>
> src/java.base/unix/native/libjava/io_util_md.c line 173:
>
>> 171: result = close(fd);
>> 172: #endif
>> 173: if (result == -1) {
>
> This needs to be `if (result == -1 && errno != EINTR)`
Yes I already caught and fixed that gaff.
>
> -
>
> PR: https://git.openjdk.java.net/jdk/pull/2173
Re: RFR: 8259943: FileDescriptor.close0 does not handle EINTR [v2]
> Please review this small change to handle `EINTR` from `close()` in > `fileDescriptorClose()`. The function `handleGetLength()` is also changed to > handle `EINTR` from `fstat64()` to match other uses of `fstat64()` in the > file. Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision: 8259943: Suppress RESTARTABLE for non-AIX. - Changes: - all: https://git.openjdk.java.net/jdk/pull/2173/files - new: https://git.openjdk.java.net/jdk/pull/2173/files/047dac0b..d351118a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2173=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2173=00-01 Stats: 5 lines in 1 file changed: 5 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2173.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2173/head:pull/2173 PR: https://git.openjdk.java.net/jdk/pull/2173
Re: RFR: 8259943: FileDescriptor.close0 does not handle EINTR
On Thu, 21 Jan 2021 07:32:39 GMT, Alan Bateman wrote:
>> Please review this small change to handle `EINTR` from `close()` in
>> `fileDescriptorClose()`. The function `handleGetLength()` is also changed to
>> handle `EINTR` from `fstat64()` to match other uses of `fstat64()` in the
>> file.
>
> src/java.base/unix/native/libjava/io_util_md.c line 167:
>
>> 165: } else {
>> 166: int result;
>> 167: RESTARTABLE(close(fd), result);
>
> close is not restartable (except AIX apparently). This means you have to
> explicitly handle EINTR, see UnixNativeDispatcher.close0 for an example.
I had it that way before creating the PR; will revert.
-
PR: https://git.openjdk.java.net/jdk/pull/2173
Re: RFR: 8259943: FileDescriptor.close0 does not handle EINTR
On Wed, 20 Jan 2021 23:01:19 GMT, Brian Burkhalter wrote:
> Please review this small change to handle `EINTR` from `close()` in
> `fileDescriptorClose()`. The function `handleGetLength()` is also changed to
> handle `EINTR` from `fstat64()` to match other uses of `fstat64()` in the
> file.
src/java.base/unix/native/libjava/io_util_md.c line 167:
> 165: } else {
> 166: int result;
> 167: RESTARTABLE(close(fd), result);
close is not restartable (except AIX apparently). This means you have to
explicitly handle EINTR, see UnixNativeDispatcher.close0 for an example.
-
PR: https://git.openjdk.java.net/jdk/pull/2173
Re: RFR: 8259943: FileDescriptor.close0 does not handle EINTR
On Wed, 20 Jan 2021 23:01:19 GMT, Brian Burkhalter wrote: > Please review this small change to handle `EINTR` from `close()` in > `fileDescriptorClose()`. The function `handleGetLength()` is also changed to > handle `EINTR` from `fstat64()` to match other uses of `fstat64()` in the > file. Looks good, Brian. - Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2173
RFR: 8259943: FileDescriptor.close0 does not handle EINTR
Please review this small change to handle `EINTR` from `close()` in `fileDescriptorClose()`. The function `handleGetLength()` is also changed to handle `EINTR` from `fstat64()` to match other uses of `fstat64()` in the file. - Commit messages: - 8259943: FileDescriptor.close0 does not handle EINTR Changes: https://git.openjdk.java.net/jdk/pull/2173/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2173=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8259943 Stats: 10 lines in 1 file changed: 6 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/2173.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2173/head:pull/2173 PR: https://git.openjdk.java.net/jdk/pull/2173
