[checkmk-commits] Ignore the dreaded leaking file descriptor problem for now.

Module: check_mk Branch: master Commit: 8a9a8c65f3520521b0f288a331c00d62cbfef1e4 URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=8a9a8c65f35205… Author: Sven Panne <sp(a)mathias-kettner.de> Date: Thu Dec 7 12:08:30 2017 +0100 Ignore the dreaded leaking file descriptor problem for now. The problem of leaking file descriptors across fork()/exec() is a dark and heavily disputed corner of many *nices, including Linux. In a nutshell: It's a horrible design flaw, and there are basically two approaches of working around it: * First workaround: You directly open *all* your files with O_CLOEXEC, but to do that atomically, you need a bunch of new system calls. Naively using "fcntl(fd, F_SETFD, FD_CLOEXEC)" introduces a race condition during the time between open() and fcntl() where a fork()/exec() may happen. In addition you'll have various kinds of fun trying to dup()/dup2()/dup3() your file descriptors correctly. * Second workaround: Explicitly close all file descriptors you don't need anymore before a fork()/exec(). This has various downsides, too: You don't know exactly which file descriptors are open (think: libraries!), and which of them should survive. Furhtermore, you can only pray that your libraries either don't fork()/exec() or that they use the same workaround as you. Note that both approaches only really work if all SW components use the same workaround, which is *far* from reality. Just a few entertaining references to see that this is not a problem which is easy to solve and where everybody agrees: * glibc bug 10353 "Methods for deleting all file descriptors greater than given integer" (https://sourceware.org/bugzilla/show_bug.cgi?id=10353): Several people request glibc support for the 2nd workaround, but Ulrich Drepper is as "friendly" and "open-minded" as usual and insists on the 1st workaround, ignoring its inherent problems... :-P * Python issue 21207 "urandom persistent fd - not re-openned after fd close" (https://bugs.python.org/issue21207): Some library uses the 2nd workaround, but Python is not prepared for that. * "Excuse me son, but your code is leaking !!!" (http://danwalsh.livejournal.com/53603.html): One of many blog entries advertising the first workaround. * "I'm done with O_CLOEXEC" (https://mail.gnome.org/archives/gtk-devel-list/2015-March/msg00038.html): A rant from a core GNOME developer, saying goodbye to the 1st workaround. As a fun fact, ancient CentOS 5 ships with Linux 2.6.18 from 2006, but O_CLOEXEC was introduced only in 2.6.23 from 2007. Furthermore, the corresponding "e" modifier for fopen() was introduced in glibc 2.7 (2007), but CentOS 5 ships with the brand-new, bleeding-edge glibc version 2.5 from 2006. :-/ So we can't use the 1st workaround yet, only when we drop support for CentOS 5. What the NEB/CMC currently does is a mixture of both workarounds, including some hopefully harmless race conditions. This should be good enough for now, but we should definitely come back to that dark corner at some point in time. Change-Id: Ic7f78e13acc866cea81ec87bb92fc58cd812d3a1 --- .clang-tidy | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.clang-tidy b/.clang-tidy index ec71d61..aeabfec 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -15,6 +15,8 @@ # * llvm-include-order ("#includes are not sorted properly") # # * desirable checks, but hard to fix currently: +# * android-cloexec-fopen ("use 'fopen' mode 'e' to set O_CLOEXEC") Ancient CentOS 5 doesn't have O_CLOEXEC +# * android-cloexec-open ("'open' should use O_CLOEXEC where possible") Ancient CentOS 5 doesn't have O_CLOEXEC # * cert-env33-c ("calling 'system' uses a command processor") # * cert-err34-c ("'atoi' used to convert a string to an integer value, but function will not report conversion errors; consider using 'strtol' instead") # * cppcoreguidelines-pro-type-member-init ("uninitialized record type: 'foo'") @@ -27,7 +29,7 @@ # * cppcoreguidelines-pro-type-union-access ("do not access members of unions; use (boost::)variant instead") # * modernize-use-bool-literals ("converting integer literal to bool, use bool literal instead") Incorrectly triggered by e.g. FD_ZERO # -Checks: '*,-cert-env33-c,-cert-err34-c,-cert-err58-cpp,-clang-analyzer-alpha*,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-cppcoreguidelines-pro-bounds-constant-array-index,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-cppcoreguidelines-pro-type-const-cast,-cppcoreguidelines-pro-type-member-init,-cppcoreguidelines-pro-type-reinterpret-cast,-cppcoreguidelines-pro-type-static-cast-downcast,-cppcoreguidelines-pro-type-union-access,-cppcoreguidelines-pro-type-vararg,-google-runtime-int,-google-runtime-references,-hicpp-member-init,-hicpp-no-assembler,-llvm-include-order,-modernize-use-bool-literals' +Checks: '*,-android-cloexec-fopen,-android-cloexec-open,-cert-env33-c,-cert-err34-c,-cert-err58-cpp,-clang-analyzer-alpha*,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-cppcoreguidelines-pro-bounds-constant-array-index,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-cppcoreguidelines-pro-type-const-cast,-cppcoreguidelines-pro-type-member-init,-cppcoreguidelines-pro-type-reinterpret-cast,-cppcoreguidelines-pro-type-static-cast-downcast,-cppcoreguidelines-pro-type-union-access,-cppcoreguidelines-pro-type-vararg,-google-runtime-int,-google-runtime-references,-hicpp-member-init,-hicpp-no-assembler,-llvm-include-order,-modernize-use-bool-literals' CheckOptions: - key: google-readability-namespace-comments.SpacesBeforeComments value: '1'