Reduce UDS/segment name clashes across same-service instances (#2023)

Add a PID file name hash to the names of the shared memory segments and
Unix Domain Sockets. Since all instances running on the same host are
supposed to have unique PID files, this addition significantly reduces
the probability of name clashes when running multiple Squid instances
with the same service name (i.e. the same `squid -n` parameter value
that defaults to "squid").

A clash may still happen if two different PID file names have the same
hash or if multiple instances disable PID file management with
`pid_filename none`. Clashes may also happen in environments where Squid
does not even use service name for naming shared memory segments.

Examples of UDS and shared memory segment names (while using default
service name):

    /var/run/squid/squid-SLWQ-kid-1.ipc
    /var/run/squid/squid-SLWQ-coordinator.ipc
    /dev/shm/squid-SLWQ-tls_session_cache.shm
    /dev/shm/squid-SLWQ-transients_map_slices.shm

This change is a reference point for automated CONTRIBUTORS updates.

Author: ankor2023

CONTRIBUTORS

@@ -54,7 +54,7 @@ Thank you!
     Andrew Tridgell
     Andrey <[email protected]>
     Andrey Shorin <[email protected]>
-    ankor2023 <138755079+ankor2023@users.noreply.github.com>
+    Ankor <ankor2023@gmail.com>
     Anonymous <[email protected]>
     Anonymous <[email protected]>
     Anonymous Pootle User

src/Instance.cc

@@ -11,6 +11,7 @@
 #include "debug/Messages.h"
 #include "fs_io.h"
 #include "Instance.h"
+#include "md5.h"
 #include "parser/Tokenizer.h"
 #include "sbuf/Stream.h"
 #include "SquidConfig.h"
@@ -222,3 +223,43 @@ Instance::WriteOurPid()
     debugs(50, Important(23), "Created " << TheFile);
 }
 
+/// A hash that is likely to be unique across instances running on the same host
+/// because such concurrent instances should use unique PID filenames.
+/// All instances with disabled PID file maintenance have the same hash value.
+/// \returns a 4-character string suitable for use in file names and similar contexts
+static SBuf
+PidFilenameHash()
+{
+    uint8_t hash[SQUID_MD5_DIGEST_LENGTH];
+
+    SquidMD5_CTX ctx;
+    SquidMD5Init(&ctx);
+    const auto name = PidFilenameCalc();
+    SquidMD5Update(&ctx, name.rawContent(), name.length());
+    SquidMD5Final(hash, &ctx);
+
+    // converts raw hash byte at a given position to a filename-suitable character
+    const auto hashAt = [&hash](const size_t idx) {
+        const auto safeChars = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
+        return safeChars[hash[idx] % strlen(safeChars)];
+    };
+
+    SBuf buf;
+    buf.appendf("%c%c%c%c", hashAt(0), hashAt(1), hashAt(2), hashAt(3));
+    return buf;
+}
+
+SBuf
+Instance::NamePrefix(const char * const head, const char * const tail)
+{
+    SBuf buf(head);
+    buf.append(service_name);
+    buf.append("-");
+    buf.append(PidFilenameHash());
+    if (tail) {
+        // TODO: Remove leading "-" from callers and explicitly add it here.
+        buf.append(tail);
+    }
+    return buf;
+}
+

src/Instance.h

@@ -9,6 +9,8 @@
 #ifndef SQUID_SRC_INSTANCE_H
 #define SQUID_SRC_INSTANCE_H
 
+#include "sbuf/forward.h"
+
 #if HAVE_SYS_TYPES_H
 #include <sys/types.h>
 #endif
@@ -30,6 +32,14 @@ void WriteOurPid();
 /// Throws if PID file maintenance is disabled.
 pid_t Other();
 
+/// A service_name-derived string that is likely to be unique across all Squid
+/// instances concurrently running on the same host (as long as they do not
+/// disable PID file maintenance).
+/// \param head is used at the beginning of the generated name
+/// \param tail is used at the end of the generated name (when not nil)
+/// \returns a head-...tail string suitable for making file and shm segment names
+SBuf NamePrefix(const char *head, const char *tail = nullptr);
+
 } // namespace Instance
 
 #endif /* SQUID_SRC_INSTANCE_H */

src/Makefile.am

@@ -1032,6 +1032,7 @@ tests_testRock_SOURCES = \
 	HttpHeaderTools.h \
 	HttpReply.cc \
 	tests/stub_HttpRequest.cc \
+	tests/stub_Instance.cc \
 	LogTags.cc \
 	MasterXaction.cc \
 	MasterXaction.h \
@@ -1202,6 +1203,7 @@ tests_testUfs_SOURCES = \
 	HttpHeaderTools.h \
 	HttpReply.cc \
 	tests/stub_HttpRequest.cc \
+	tests/stub_Instance.cc \
 	LogTags.cc \
 	MasterXaction.cc \
 	MasterXaction.h \
@@ -1374,6 +1376,7 @@ tests_testStore_SOURCES = \
 	HttpHeaderTools.h \
 	tests/stub_HttpReply.cc \
 	tests/stub_HttpRequest.cc \
+	tests/stub_Instance.cc \
 	MasterXaction.cc \
 	MasterXaction.h \
 	MemBuf.cc \
@@ -1537,6 +1540,7 @@ tests_testDiskIO_SOURCES = \
 	HttpHeaderTools.h \
 	HttpReply.cc \
 	tests/stub_HttpRequest.cc \
+	tests/stub_Instance.cc \
 	LogTags.cc \
 	MasterXaction.cc \
 	MasterXaction.h \
@@ -1800,6 +1804,7 @@ tests_testHttpRange_SOURCES = \
 	HttpReply.cc \
 	HttpRequest.cc \
 	tests/stub_HttpUpgradeProtocolAccess.cc \
+	tests/stub_Instance.cc \
 	IoStats.h \
 	tests/stub_IpcIoFile.cc \
 	LogTags.cc \
@@ -2060,6 +2065,7 @@ tests_testHttpReply_SOURCES = \
 	tests/testHttpReply.cc \
 	HttpReply.h \
 	tests/stub_HttpRequest.cc \
+	tests/stub_Instance.cc \
 	MasterXaction.cc \
 	MasterXaction.h \
 	MemBuf.cc \
@@ -2187,6 +2193,7 @@ tests_testHttpRequest_SOURCES = \
 	tests/testHttpRequest.cc \
 	tests/testHttpRequestMethod.cc \
 	tests/stub_HttpUpgradeProtocolAccess.cc \
+	tests/stub_Instance.cc \
 	IoStats.h \
 	tests/stub_IpcIoFile.cc \
 	LogTags.cc \
@@ -2484,6 +2491,7 @@ tests_testCacheManager_SOURCES = \
 	HttpReply.cc \
 	HttpRequest.cc \
 	tests/stub_HttpUpgradeProtocolAccess.cc \
+	tests/stub_Instance.cc \
 	IoStats.h \
 	tests/stub_IpcIoFile.cc \
 	LogTags.cc \

src/ipc/Port.cc

@@ -13,6 +13,7 @@
 #include "comm/Connection.h"
 #include "comm/Read.h"
 #include "CommCalls.h"
+#include "Instance.h"
 #include "ipc/Port.h"
 #include "sbuf/Stream.h"
 #include "tools.h"
@@ -52,9 +53,8 @@ bool Ipc::Port::doneAll() const
 String Ipc::Port::MakeAddr(const char* processLabel, int id)
 {
     assert(id >= 0);
-    String addr = channelPathPfx;
-    addr.append(service_name);
-    addr.append(processLabel);
+    String addr;
+    addr.append(Instance::NamePrefix(channelPathPfx, processLabel));
     addr.append('-');
     addr.append(xitoa(id));
     addr.append(".ipc");
@@ -66,9 +66,7 @@ Ipc::Port::CoordinatorAddr()
 {
     static String coordinatorAddr;
     if (!coordinatorAddr.size()) {
-        coordinatorAddr= channelPathPfx;
-        coordinatorAddr.append(service_name);
-        coordinatorAddr.append(coordinatorAddrLabel);
+        coordinatorAddr.append(Instance::NamePrefix(channelPathPfx, coordinatorAddrLabel));
         coordinatorAddr.append(".ipc");
     }
     return coordinatorAddr;

src/ipc/mem/Segment.cc

@@ -13,6 +13,7 @@
 #include "compat/shm.h"
 #include "debug/Stream.h"
 #include "fatal.h"
+#include "Instance.h"
 #include "ipc/mem/Segment.h"
 #include "sbuf/SBuf.h"
 #include "SquidConfig.h"
@@ -277,8 +278,7 @@ Ipc::Mem::Segment::GenerateName(const char *id)
         if (name[name.size()-1] != '/')
             name.append('/');
     } else {
-        name.append('/');
-        name.append(service_name);
+        name.append(Instance::NamePrefix("/"));
         name.append('-');
     }
 

src/tests/Stub.am

@@ -22,6 +22,7 @@ STUB_SOURCE = \
 	tests/stub_HttpReply.cc \
 	tests/stub_HttpRequest.cc \
 	tests/stub_HttpUpgradeProtocolAccess.cc \
+	tests/stub_Instance.cc \
 	tests/stub_IpcIoFile.cc \
 	tests/stub_MemBuf.cc \
 	tests/stub_MemObject.cc \

src/tests/stub_Instance.cc

@@ -0,0 +1,20 @@
+/*
+ * Copyright (C) 1996-2025 The Squid Software Foundation and contributors
+ *
+ * Squid software is distributed under GPLv2+ license and includes
+ * contributions from numerous individuals and organizations.
+ * Please see the COPYING and CONTRIBUTORS files for details.
+ */
+
+#include "squid.h"
+#include "Instance.h"
+#include "sbuf/SBuf.h"
+
+#define STUB_API "Instance.cc"
+#include "tests/STUB.h"
+
+void Instance::ThrowIfAlreadyRunning() STUB
+void Instance::WriteOurPid() STUB
+pid_t Instance::Other() STUB_RETVAL({})
+SBuf Instance::NamePrefix(const char *, const char *) STUB_RETVAL_NOP(SBuf("squid-0"))
+