Alexey Serbin has uploaded this change for review. (
http://gerrit.cloudera.org:8080/22521
Change subject: KUDU-3645 do not retry close() syscall on EINTR
......................................................................
KUDU-3645 do not retry close() syscall on EINTR
It's not a good idea to retry calling close() on EINTR, at least
on Linux and other Unices except for HP-UX. Since Kudu isn't officially
supported on HP-UX, this patch removes the retry of close() from all the
applicable call sites where it was blanket-wrapped with the
RETRY_ON_EINTR macro.
The same issue has been addressed in the Boost library as well [1][2].
It's a well known issue and it's been discussed a long time ago [3].
Also, it's well documented in the manual page [4]:
The EINTR error is a somewhat special case. Regarding the EINTR
error, POSIX.1-2008 says:
If close() is interrupted by a signal that is to be caught,
it shall return -1 with errno set to EINTR and the state of
fildes is unspecified.
This permits the behavior that occurs on Linux and many other
implementations, where, as with other errors that may be reported
by close(), the file descriptor is guaranteed to be closed.
However, it also permits another possibility: that the
implementation returns an EINTR error and keeps the file
descriptor open. (According to its documentation, HP-UX's close()
does this.) The caller must then once more use close() to close
the file descriptor, to avoid file descriptor leaks. This
divergence in implementation behaviors provides a difficult hurdle
for portable applications, since on many implementations, close()
must not be called again after an EINTR error, and on at least
one, close() must be called again. There are plans to address
this conundrum for the next major release of the POSIX.1 standard.
[1] https://github.com/boostorg/beast/issues/1445
[2] https://github.com/boostorg/beast/commit/0ce8ebbef
[3] https://lwn.net/Articles/576478/
[4] https://man7.org/linux/man-pages/man2/close.2.html
Change-Id: I938e487bd937ef6ee8764b9aa88cb5f7a2c33158
---
M src/kudu/gutil/sysinfo.cc
M src/kudu/subprocess/subprocess_protocol.cc
M src/kudu/util/env-test.cc
M src/kudu/util/env_posix.cc
M src/kudu/util/net/diagnostic_socket.cc
M src/kudu/util/net/socket.cc
M src/kudu/util/pstack_watcher-test.cc
M src/kudu/util/pstack_watcher.cc
M src/kudu/util/subprocess-test.cc
M src/kudu/util/subprocess.cc
10 files changed, 139 insertions(+), 104 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/22521/1
--
To view, visit http://gerrit.cloudera.org:8080/22521
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I938e487bd937ef6ee8764b9aa88cb5f7a2c33158
Gerrit-Change-Number: 22521
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <[email protected]>
