Windows: Fix error displaying helper name on pipe close errors

The helper name string is not directly available to the HelperServerBase
methods for closing pipes. Pass it from the method callers as needed and
drop logging of the command line details.

diff --git a/src/helper.cc b/src/helper.cc
index b35c43cef42d9cd199f450efc308a0643ac6a1ba..c07787e0c1450123bed0012fe9622eb181af625e 100644 (file)
--- a/src/helper.cc
+++ b/src/helper.cc
@@ -76,7 +76,7 @@ HelperServerBase::initStats()
 }
 
 void
-HelperServerBase::closePipesSafely()
+HelperServerBase::closePipesSafely(const char *id_name)
 {
 #if _SQUID_WINDOWS_
     shutdown(writePipe->fd, SD_BOTH);
@@ -93,9 +93,8 @@ HelperServerBase::closePipesSafely()
     if (hIpc) {
         if (WaitForSingleObject(hIpc, 5000) != WAIT_OBJECT_0) {
             getCurrentTime();
-            debugs(84, DBG_IMPORTANT, "WARNING: " << hlp->id_name <<
-                   " #" << index << " (" << hlp->cmdline->key << "," <<
-                   (long int)pid << ") didn't exit in 5 seconds");
+            debugs(84, DBG_IMPORTANT, "WARNING: " << id_name <<
+                   " #" << index << " (PID " << (long int)pid << ") didn't exit in 5 seconds");
         }
         CloseHandle(hIpc);
     }
@@ -103,7 +102,7 @@ HelperServerBase::closePipesSafely()
 }
 
 void
-HelperServerBase::closeWritePipeSafely()
+HelperServerBase::closeWritePipeSafely(const char *id_name)
 {
 #if _SQUID_WINDOWS_
     shutdown(writePipe->fd, (readPipe->fd == writePipe->fd ? SD_BOTH : SD_SEND));
@@ -118,9 +117,8 @@ HelperServerBase::closeWritePipeSafely()
     if (hIpc) {
         if (WaitForSingleObject(hIpc, 5000) != WAIT_OBJECT_0) {
             getCurrentTime();
-            debugs(84, DBG_IMPORTANT, "WARNING: " << hlp->id_name <<
-                   " #" << index << " (" << hlp->cmdline->key << "," <<
-                   (long int)pid << ") didn't exit in 5 seconds");
+            debugs(84, DBG_IMPORTANT, "WARNING: " << id_name <<
+                   " #" << index << " (PID " << (long int)pid << ") didn't exit in 5 seconds");
         }
         CloseHandle(hIpc);
     }
@@ -612,7 +610,7 @@ helperShutdown(helper * hlp)
         /* the rest of the details is dealt with in the helperServerFree
          * close handler
          */
-        srv->closePipesSafely();
+        srv->closePipesSafely(hlp->id_name);
     }
 }
 
@@ -659,7 +657,7 @@ helperStatefulShutdown(statefulhelper * hlp)
         /* the rest of the details is dealt with in the helperStatefulServerFree
          * close handler
          */
-        srv->closePipesSafely();
+        srv->closePipesSafely(hlp->id_name);
     }
 }
 
@@ -700,7 +698,7 @@ helperServerFree(helper_server *srv)
     }
 
     if (Comm::IsConnOpen(srv->writePipe))
-        srv->closeWritePipeSafely();
+        srv->closeWritePipeSafely(hlp->id_name);
 
     dlinkDelete(&srv->link, &hlp->servers);
 
@@ -768,7 +766,7 @@ helperStatefulServerFree(helper_stateful_server *srv)
 
     /* TODO: walk the local queue of requests and carry them all out */
     if (Comm::IsConnOpen(srv->writePipe))
-        srv->closeWritePipeSafely();
+        srv->closeWritePipeSafely(hlp->id_name);
 
     dlinkDelete(&srv->link, &hlp->servers);
 
@@ -883,7 +881,7 @@ helperHandleRead(const Comm::ConnectionPointer &conn, char *buf, size_t len, Com
     debugs(84, 5, "helperHandleRead: " << len << " bytes from " << hlp->id_name << " #" << srv->index);
 
     if (flag != Comm::OK || len == 0) {
-        srv->closePipesSafely();
+        srv->closePipesSafely(hlp->id_name);
         return;
     }
 
@@ -948,7 +946,7 @@ helperHandleRead(const Comm::ConnectionPointer &conn, char *buf, size_t len, Com
             debugs(84, DBG_IMPORTANT, "ERROR: Disconnecting from a " <<
                    "helper that overflowed " << srv->rbuf_sz << "-byte " <<
                    "Squid input buffer: " << hlp->id_name << " #" << srv->index);
-            srv->closePipesSafely();
+            srv->closePipesSafely(hlp->id_name);
             return;
         }
 
@@ -979,7 +977,7 @@ helperStatefulHandleRead(const Comm::ConnectionPointer &conn, char *buf, size_t
            hlp->id_name << " #" << srv->index);
 
     if (flag != Comm::OK || len == 0) {
-        srv->closePipesSafely();
+        srv->closePipesSafely(hlp->id_name);
         return;
     }
 
@@ -1068,7 +1066,7 @@ helperStatefulHandleRead(const Comm::ConnectionPointer &conn, char *buf, size_t
             debugs(84, DBG_IMPORTANT, "ERROR: Disconnecting from a " <<
                    "helper that overflowed " << srv->rbuf_sz << "-byte " <<
                    "Squid input buffer: " << hlp->id_name << " #" << srv->index);
-            srv->closePipesSafely();
+            srv->closePipesSafely(hlp->id_name);
             return;
         }
 
@@ -1412,7 +1410,7 @@ helperStatefulServerDone(helper_stateful_server * srv)
     if (!srv->flags.shutdown) {
         helperStatefulKickQueue(srv->parent);
     } else if (!srv->flags.closing && !srv->flags.reserved && !srv->flags.busy) {
-        srv->closeWritePipeSafely();
+        srv->closeWritePipeSafely(srv->parent->id_name);
         return;
     }
 }

diff --git a/src/helper.h b/src/helper.h
index b828b18938146bda7c9c9e41bbdcd2c280a5b6f8..c0454d2c493a481310c779209970cc439a8fe155 100644 (file)
--- a/src/helper.h
+++ b/src/helper.h
@@ -84,14 +84,18 @@ class HelperServerBase
 public:
     /** Closes pipes to the helper safely.
      * Handles the case where the read and write pipes are the same FD.
+     *
+     * \param name displayed for the helper being shutdown if logging an error
      */
-    void closePipesSafely();
+    void closePipesSafely(const char *name);
 
     /** Closes the reading pipe.
      * If the read and write sockets are the same the write pipe will
      * also be closed. Otherwise its left open for later handling.
+     *
+     * \param name displayed for the helper being shutdown if logging an error
      */
-    void closeWritePipeSafely();
+    void closeWritePipeSafely(const char *name);
 
 public:
     /// Helper program identifier; does not change when contents do,