Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions compat/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ libcompatsquid_la_SOURCES = \
select.h \
shm.cc \
shm.h \
signal.cc \
signal.h \
socket.cc \
socket.h \
statvfs.cc \
Expand Down
46 changes: 0 additions & 46 deletions compat/mswindows.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,52 +61,6 @@ chroot(const char *dirname)
return GetLastError();
}

void
GetProcessName(pid_t pid, char *ProcessName)
{
strcpy(ProcessName, "unknown");
#if defined(PSAPI_VERSION)
/* Get a handle to the process. */
HANDLE hProcess = OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, FALSE, pid);
/* Get the process name. */
if (hProcess) {
HMODULE hMod;
DWORD cbNeeded;

if (EnumProcessModules(hProcess, &hMod, sizeof(hMod), &cbNeeded))
GetModuleBaseName(hProcess, hMod, ProcessName, sizeof(ProcessName));
else {
CloseHandle(hProcess);
return;
}
} else
return;
CloseHandle(hProcess);
#endif
}

int
kill(pid_t pid, int sig)
{
HANDLE hProcess;
char MyProcessName[MAX_PATH];
char ProcessNameToCheck[MAX_PATH];

if (sig == 0) {
if (!(hProcess = OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, FALSE, pid)))
return -1;
else {
CloseHandle(hProcess);
GetProcessName(getpid(), MyProcessName);
GetProcessName(pid, ProcessNameToCheck);
if (strcmp(MyProcessName, ProcessNameToCheck) == 0)
return 0;
return -1;
}
} else
return 0;
}

#if !HAVE_GETTIMEOFDAY
int
gettimeofday(struct timeval *pcur_time, void *tzp)
Expand Down
9 changes: 0 additions & 9 deletions compat/os/mswindows.h
Original file line number Diff line number Diff line change
Expand Up @@ -220,14 +220,6 @@ setenv(const char * const name, const char * const value, const int overwrite)
#define S_ISDIR(m) (((m) & _S_IFDIR) == _S_IFDIR)
#endif

#define SIGHUP 1 /* hangup */
#define SIGKILL 9 /* kill (cannot be caught or ignored) */
#define SIGBUS 10 /* bus error */
#define SIGPIPE 13 /* write on a pipe with no one to read it */
#define SIGCHLD 20 /* to parent on child stop or exit */
#define SIGUSR1 30 /* user defined signal 1 */
#define SIGUSR2 31 /* user defined signal 2 */

#if defined(_MSC_VER)
typedef int uid_t;
typedef int gid_t;
Expand Down Expand Up @@ -534,7 +526,6 @@ struct rusage {
#undef ACL

SQUIDCEXTERN int chroot(const char *dirname);
SQUIDCEXTERN int kill(pid_t, int);
SQUIDCEXTERN struct passwd * getpwnam(char *unused);
SQUIDCEXTERN struct group * getgrnam(char *unused);

Expand Down
82 changes: 82 additions & 0 deletions compat/signal.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/*
* 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 "compat/signal.h"

#if _SQUID_WINDOWS_ || _SQUID_MINGW_

#ifdef HAVE_PSAPI_H
#include <psapi.h>
#endif

static void
GetProcessName(pid_t pid, char *ProcessName)
{
strcpy(ProcessName, "unknown");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this code survives, please return a nil name instead of using a special value to mean "we do not know what the process name is [because some system calls have failed]" (that callers do not know about!).

N.B. I am aware that official code uses this hack.


auto hProcess = OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, FALSE, pid);
if (!hProcess) // If we cannot open the process, we cannot get its name.
return;

/* Get the process name. */
HMODULE hMod;
DWORD cbNeeded;

if (EnumProcessModules(hProcess, &hMod, sizeof(hMod), &cbNeeded)) {
GetModuleBaseName(hProcess, hMod, ProcessName, sizeof(ProcessName));
}
CloseHandle(hProcess);
}

/// true if the given pid is the current process, false otherwise
bool
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the caller is restored to using xkill(pid, 0) to check for validity this can be made static

Suggested change
bool
static bool

IsPidValid(pid_t pid)
{
char MyProcessName[MAX_PATH];
GetProcessName(getpid(), MyProcessName);
char ProcessNameToCheck[MAX_PATH];
Comment on lines +41 to +43
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAX_PATH is not always a small value. Please allocate these dynamically.

GetProcessName(pid, ProcessNameToCheck);
if (strcmp(MyProcessName, ProcessNameToCheck) == 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not clear to me why IsPidValid(getpid()) should return false. It is usually possible to send a signal to oneself, right? Please either refactor or add a C++ comment to explain why we need to compare getpid() process name with ours.

N.B. I am aware that existing Windows code has similar logic.

P.S. I am writing this change request while assuming that this or similar function is going to be preserved despite the requested removal of "public" IsPidValid() API.

return true;
return false;
}

int
xkill(pid_t pid, int sig)
{
auto hProcess = OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, FALSE, pid);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

if (!hProcess)
return -1;

switch (sig) {
// Windows does not support sending generic POSIX signals.
// TODO: implement a signal sending and handling mechanism
// using Windows messages
case SIGKILL:
if (TerminateProcess(hProcess, 0)) {
CloseHandle(hProcess);
return 0;
}
break;
case 0:
CloseHandle(hProcess);
if (IsPidValid(pid))
return 0;
return -1;
break;
default:
// Unsupported signal
CloseHandle(hProcess);
}

return -1;
Comment on lines +58 to +79
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The break; case for SIGKILL handling leaves the process handle hanging.

This whole section would be better implemented such that the handle is always closed at a single point of return.

Suggested change
switch (sig) {
// Windows does not support sending generic POSIX signals.
// TODO: implement a signal sending and handling mechanism
// using Windows messages
case SIGKILL:
if (TerminateProcess(hProcess, 0)) {
CloseHandle(hProcess);
return 0;
}
break;
case 0:
CloseHandle(hProcess);
if (IsPidValid(pid))
return 0;
return -1;
break;
default:
// Unsupported signal
CloseHandle(hProcess);
}
return -1;
// Windows does not support sending generic POSIX signals.
// TODO: implement a signal sending and handling mechanism
// using Windows messages
bool result = false;
switch (sig) {
case SIGKILL:
result = TerminateProcess(hProcess, 0);
break;
case 0:
result = IsPidValid(pid);
break;
default:
// Unsupported signal
}
CloseHandle(hProcess);
return (result ? 0 : -1);

}

#endif /* _SQUID_WINDOWS_ || _SQUID_MINGW_ */
87 changes: 87 additions & 0 deletions compat/signal.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/*
* 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.
*/

#ifndef SQUID_COMPAT_SIGNAL_H
#define SQUID_COMPAT_SIGNAL_H

#if !defined(SIGHUP)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a reference for where these values come from.

Suggested change
#if !defined(SIGHUP)
// Signal definitions from POSIX API. see https://www.man7.org/linux/man-pages/man7/signal.7.html
// TODO: use x86 or arch-specific defines instead of these SPARC CPU values.
#if !defined(SIGHUP)

#define SIGHUP 1 /* hangup */
#endif
#if !defined(SIGKILL)
#define SIGKILL 9 /* kill (cannot be caught or ignored) */
#endif
#if !defined(SIGBUS)
#define SIGBUS 10 /* bus error */
#endif
#if !defined(SIGPIPE)
#define SIGPIPE 13 /* write on a pipe with no one to read it */
#endif
#if !defined(SIGALRM)
#define SIGALRM 14 /* real-time timer expired */
#endif
#if !defined(SIGCHLD)
#define SIGCHLD 20 /* to parent on child stop or exit */
#endif
#if !defined(SIGUSR1)
#define SIGUSR1 30 /* user defined signal 1 */
#endif
#if !defined(SIGUSR2)
#define SIGUSR2 31 /* user defined signal 2 */
#endif

/// POSIX kill(2) equivalent
int xkill(pid_t pid, int sig);

/// true if pid can be sent a signal (no signal is sent)
inline bool
IsPidValid(pid_t pid);

Comment on lines +40 to +43
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not part of the POSIX API. We should be using that API instead of local wrappers.

Suggested change
/// true if pid can be sent a signal (no signal is sent)
inline bool
IsPidValid(pid_t pid);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that compat/signal.h should not provide IsPidValid() API.

Instead, xkill() implementation for Windows may (and probably should!) use a static helper function, hopefully with a more revealing name, to implement xkill(signal=0) support).

Old higher-level code should continue to use ProcessIsRunning() and raw xkill(signal=0) calls.

#if !defined(WIFEXITED)
inline int
WIFEXITED(int status) {
return (status & 0x7f) == 0;
}
#endif

#if !defined(WEXITSTATUS)
inline int
WEXITSTATUS(int status) {
return (status & 0xff00) >> 8;
}
#endif

#if !defined(WIFSIGNALED)
inline int
WIFSIGNALED(int status) {
return (status & 0x7f) != 0;
}
#endif

#if !defined(WTERMSIG)
inline int
WTERMSIG(int status) {
return (status & 0x7f);
}
#endif
Comment on lines +44 to +70
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All these W...() definitions are part of the POSIX sys/wait.h header. Not part of signal.h.
Please remove.


#if !(_SQUID_WINDOWS_ || _SQUID_MINGW_)

inline int xkill(pid_t pid, int sig)
{
return kill(pid, sig);
}

inline bool
IsPidValid(pid_t pid)
{
return kill(pid, 0) == 0;
}

#endif /* !(_SQUID_WINDOWS_ || _SQUID_MINGW_) */
Comment on lines +72 to +85
Copy link
Contributor

@yadij yadij Dec 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#if !(_SQUID_WINDOWS_ || _SQUID_MINGW_)
inline int xkill(pid_t pid, int sig)
{
return kill(pid, sig);
}
inline bool
IsPidValid(pid_t pid)
{
return kill(pid, 0) == 0;
}
#endif /* !(_SQUID_WINDOWS_ || _SQUID_MINGW_) */
/// POSIX kill(2) equivalent
int xkill(pid_t, int);
#if _SQUID_WINDOWS_ || _SQUID_MINGW_
inline int xkill(pid_t pid, int sig) { return kill(pid, sig); }
#endif


#endif /* SQUID_COMPAT_SIGNAL_H */
3 changes: 2 additions & 1 deletion src/Instance.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include "squid.h"
#include "base/File.h"
#include "compat/signal.h"
#include "debug/Messages.h"
#include "fs_io.h"
#include "Instance.h"
Expand Down Expand Up @@ -95,7 +96,7 @@ GetOtherPid(File &pidFile)
static bool
ProcessIsRunning(const pid_t pid)
{
const auto result = kill(pid, 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please continue to use the POSIX API equivalent call.

    const auto result = xkill(pid, 0);

const auto result = IsPidValid(pid);
const auto savedErrno = errno;
if (result != 0)
debugs(50, 3, "kill(" << pid << ", 0) failed: " << xstrerr(savedErrno));
Expand Down
1 change: 1 addition & 0 deletions src/cache_cf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "CachePeer.h"
#include "CachePeers.h"
#include "compat/netdb.h"
#include "compat/signal.h"
#include "compat/socket.h"
#include "ConfigOption.h"
#include "ConfigParser.h"
Expand Down
3 changes: 2 additions & 1 deletion src/ipc/Coordinator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "CacheManager.h"
#include "comm.h"
#include "comm/Connection.h"
#include "compat/signal.h"
#include "compat/unistd.h"
#include "ipc/Coordinator.h"
#include "ipc/SharedListen.h"
Expand Down Expand Up @@ -289,7 +290,7 @@ void Ipc::Coordinator::broadcastSignal(int sig) const
for (SCI iter = strands_.begin(); iter != strands_.end(); ++iter) {
debugs(54, 5, "signal " << sig << " to kid" << iter->kidId <<
", PID=" << iter->pid);
kill(iter->pid, sig);
xkill(iter->pid, sig);
}
}

Expand Down
1 change: 1 addition & 0 deletions src/ipc/Kid.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
/* DEBUG: section 54 Interprocess Communication */

#include "squid.h"
#include "compat/signal.h"
#include "globals.h"
#include "ipc/Kid.h"
#include "SquidConfig.h"
Expand Down
3 changes: 2 additions & 1 deletion src/log/ModDaemon.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "squid.h"
#include "cbdata.h"
#include "comm/Loops.h"
#include "compat/signal.h"
#include "fatal.h"
#include "fde.h"
#include "globals.h"
Expand Down Expand Up @@ -261,7 +262,7 @@ logfile_mod_daemon_close(Logfile * lf)
comm_close(ll->rfd);
comm_close(ll->wfd);
}
kill(ll->pid, SIGTERM);
xkill(ll->pid, SIGTERM);
eventDelete(logfileFlushEvent, lf);
xfree(ll);
lf->data = nullptr;
Expand Down
3 changes: 2 additions & 1 deletion src/main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "client_side.h"
#include "comm.h"
#include "CommandLine.h"
#include "compat/signal.h"
#include "compat/unistd.h"
#include "ConfigParser.h"
#include "CpuAffinity.h"
Expand Down Expand Up @@ -1666,7 +1667,7 @@ sendSignal(void)
WIN32_sendSignal(opt_send_signal);
#else
const auto pid = Instance::Other();
if (kill(pid, opt_send_signal) &&
if (xkill(pid, opt_send_signal) &&
/* ignore permissions if just running check */
!(opt_send_signal == 0 && errno == EPERM)) {
const auto savedErrno = errno;
Expand Down
1 change: 1 addition & 0 deletions src/mgr/BasicActions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "squid.h"
#include "base/TextException.h"
#include "CacheManager.h"
#include "compat/signal.h"
#include "mgr/ActionCreator.h"
#include "mgr/ActionProfile.h"
#include "mgr/BasicActions.h"
Expand Down
3 changes: 2 additions & 1 deletion src/tools.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "anyp/PortCfg.h"
#include "base/Subscription.h"
#include "client_side.h"
#include "compat/signal.h"
#include "compat/unistd.h"
#include "fatal.h"
#include "fde.h"
Expand Down Expand Up @@ -423,7 +424,7 @@ BroadcastSignalIfAny(int& sig)
for (int i = TheKids.count() - 1; i >= 0; --i) {
const auto &kid = TheKids.get(i);
if (kid.running())
kill(kid.getPid(), sig);
xkill(kid.getPid(), sig);
}
}
sig = -1;
Expand Down
1 change: 1 addition & 0 deletions src/windows_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
/* Inspired by previous work by Romeo Anghelache & Eric Stern. */

#include "squid.h"
#include "compat/signal.h"
#include "debug/Stream.h"
#include "globals.h"
#include "protos.h"
Expand Down
Loading