Bug 5254, part 1: Do not leak master process' cache.log to kids (#1222)

The fork()ed kids unknowingly inherited cache.log file descriptor and,
hence, prevented the underlying file from being deleted on log rotation.

This change does not fully fix the bug because the file is still being
held open by the master process itself. This change was isolated because
it lacks bad/controversial side effects -- it is a simple step forward.

This fix may also help stop leaking kid's cache.log descriptor to
helpers, but that requires more work -- replacing descriptor duping
trick in ipcCreate() with a proper servicing of helper stderr descriptor
via Comm. For now, each helper process still keeps cache.log alive.

src/comm.h

@@ -22,7 +22,16 @@ bool comm_iocallbackpending(void); /* inline candidate */
 
 int commSetNonBlocking(int fd);
 int commUnsetNonBlocking(int fd);
+
+/// On platforms where FD_CLOEXEC is defined, close the given descriptor during
+/// a function call from the exec(3) family. Otherwise, do nothing; the platform
+/// itself may close-on-exec by default (e.g., MS Win32 is said to do that at
+/// https://devblogs.microsoft.com/oldnewthing/20111216-00/?p=8873); other
+/// platforms are unsupported. Callers that want close-on-exec behavior must
+/// call this function on all platforms and are not responsible for the outcome
+/// on platforms without FD_CLOEXEC.
 void commSetCloseOnExec(int fd);
+
 void _comm_close(int fd, char const *file, int line);
 #define comm_close(x) (_comm_close((x), __FILE__, __LINE__))
 void old_comm_reset_close(int fd);

src/comm/minimal.cc

@@ -7,6 +7,7 @@
  */
 
 #include "squid.h"
+#include "comm.h"
 #include "debug/Stream.h"
 #include "fd.h"
 
@@ -22,3 +23,12 @@ fd_close(const int fd)
     debugs(51, 3, "FD " << fd);
 }
 
+void
+commSetCloseOnExec(int)
+{
+    // This stub is needed because DebugFile sets this flag for the open
+    // cache.log file descriptor. Helpers and such must use stdout/stderr
+    // instead of opening a cache.log file. They should never reach this code.
+    assert(false);
+}
+

src/debug/debug.cc

@@ -10,6 +10,7 @@
 
 #include "squid.h"
 #include "base/TextException.h"
+#include "comm.h"
 #include "debug/Stream.h"
 #include "fatal.h"
 #include "fd.h"
@@ -762,8 +763,10 @@ DebugFile::reset(FILE *newFile, const char *newName)
     }
     file_ = newFile; // may be nil
 
-    if (file_)
+    if (file_) {
+        commSetCloseOnExec(fileno(file_));
         fd_open(fileno(file_), FD_LOG, Debug::cache_log);
+    }
 
     xfree(name);
     name = newName ? xstrdup(newName) : nullptr;