category/port: mail/spamassassin-3.4.0_16 Command 'service sa-spamd status' says spamd is not running. even though the pid file exists and contains the number of spamd process. The culprit is the newly added line command_interpreter="/usr/local/bin/perl" in /usr/local/etc/rc.d/sa-spamd file, which forces rc.subr to search shebang line in /usr/local/sbin/spamd executable.
Auto-assigned to maintainer adamw@FreeBSD.org
Over to marino. You break it, you buy it, brother :-) I had asked for your input on this change a couple weeks ago.
(In reply to Jiri Lazansky from comment #0) > The culprit is the newly added line > command_interpreter="/usr/local/bin/perl" > in /usr/local/etc/rc.d/sa-spamd file, which forces rc.subr to search shebang > line in /usr/local/sbin/spamd executable. what does this mean exactly? Are you saying /usr/local/bin/perl doesn't exist on the system?
I think what's going on is this final line in /etc/rc.subr:check_pidfile(): _find_processes $_procname ${_interpreter:-.} '-p '"$_pid" When command_interpreter is set, rc.subr wants to find a process in ps(1) with $command_interpreter in it. However, spamd still shows up in ps as /usr/local/bin/spamd. John, this change makes it impossible to check the status of, stop, or restart spamd. Everybody who's installed the update will have to manually kill spamd. Can you please back it out until this is resolved?
(In reply to Adam Weinberger from comment #4) wouldn't that explanation mean that every single rc.d script that defines command_interpreter is broken? There are many of them that define command_interpreter to "${PREFIX}/bin/perl". it seems strange that a process name would be checked when the pid is known as well. Adam, are you seeing breakage personally, or are you relying on this PR only?
I'm seeing the breakage too. Luckily, removing the command_interpreter line from rc.d/sa-spamd allows you to stop spamd. Please change it back for now before a lot of people upgrade. I agree that the behaviour of grepping ps(1) when the pidfile is known is strange, but I may be reading rc.subr wrong.
the commit didn't log because of a missing colon: Author: marino Date: Tue Jan 6 01:05:46 2015 New Revision: 376379 URL: https://svnweb.freebsd.org/changeset/ports/376379 QAT: https://qat.redports.org/buildarchive/r376379/ Log: mail/spamassassin: Revert command_interpreter fix for now There is a report that defining command_interpreter actually causes problem for spamd, athought it should not. While the reason is being investigated, the previous change has been reverted per maintainer's request. PR 196517 Modified: head/mail/spamassassin/Makefile head/mail/spamassassin/files/sa-spamd.in
Additional comment to my original post: rc.subr(8) man states: command_interpreter command is started with: #! command_interpreter [...] which results in its ps(1) command being: command_interpreter [...] command so use that string to find the PID(s) of the running command rather than command. ... /usr/local/bin/spamd changes its 'ps title' on line 2928 thus effectively erasing /usr/local/bin/perl from ps printout and breaking the condition required by rc.subr. So Comments #4 * #5 are irrelevant.
you mean the daemonize subroutine starting on line 2929? Wouldn't all daemons do something similar? I am interested in pavalos' analysis because right now it appears that either spamassassin is incorrectly incompatible with command_interpreter functionality or that the command_interpreter functionality itself has a bug in it.
(In reply to John Marino from comment #9) Exactly, the 'daemonize' subroutine changes the process title. This means that rc.subr's check_pidfile pidfile procname [interpreter] when it get the optional param 'interpreter' will look for 'interpreter' (not for 'procname'). I don't consider it as a bug in rc.subr but in wrong understanding what it does. Simply, the start-up script MUST NOT specify 'interpreter' or the daemon may not change the process title. Doing both CONFLICTS.
So what is it that rc.d/sa-spamd is doing (or not doing) on DragonflyBSD? I gather that John's original commit was intended to fix things over there?
A quick update -- Peter Avalos has determined this is the difference between FreeBSD and DragonFly. Perl's mg.c contains: #ifdef HAS_SETPROCTITLE /* The BSDs don't show the argv[] in ps(1) output, they * show a string from the process struct and provide * the setproctitle() routine to manipulate that. */ if (PL_origalen != 1) { s = SvPV_const(sv, len); # if __FreeBSD_version > 410001 /* The leading "-" removes the "perl: " prefix, * but not the "(perl) suffix from the ps(1) * output, because that's what ps(1) shows if the * argv[] is modified. */ setproctitle("-%s", s); # else /* old FreeBSDs, NetBSD, OpenBSD, anyBSD */ /* This doesn't really work if you assume that * $0 = 'foobar'; will wipe out 'perl' from the $0 * because in ps(1) output the result will be like * sprintf("perl: %s (perl)", s) * I guess this is a security feature: * one (a user process) cannot get rid of the original name. * --jhi */ setproctitle("%s", s); # endif So he's recommending that we patch perl mg.c to have the same behavior as FreeBSD, which will cause perl scripts not to need (actually can't use) command_interpreter just like FreeBSD.
I patched all three versions of perl (see https://github.com/DragonFlyBSD/DPorts/issues/129 ) so in theory there is no longer a difference between FreeBSD and DragonFly. Neither will require command_interpreter now. Closing the PR.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=1f3e72b3f6f9b79ad1aa36881ff1ab8c00daaf83 commit 1f3e72b3f6f9b79ad1aa36881ff1ab8c00daaf83 Author: Cy Schubert <cy@FreeBSD.org> AuthorDate: 2023-01-03 18:56:58 +0000 Commit: Cy Schubert <cy@FreeBSD.org> CommitDate: 2023-01-03 18:56:58 +0000 mail/spamassassin: Fix command_interpreter regression PR/196517, the rc script unable to find its .pid file, has returned. Restore SVN r376379 (691102663fb), fixing this bug again. PR: 196517 Reported by: ler mail/spamassassin/Makefile | 1 + mail/spamassassin/files/sa-spamd.in | 1 - 2 files changed, 1 insertion(+), 1 deletion(-)