Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mail/spamassassin rc.d script needs command_interpreter=/usr/local/bin/perl #129

Closed
pavalos opened this issue Jan 4, 2015 · 16 comments
Closed

Comments

@pavalos
Copy link

pavalos commented Jan 4, 2015

mail/spamassassin installs an rc.d script (sa-spamd), but restart doesn't work since spamd is actually a perl script. Adding "command_interpreter=/usr/local/bin/perl" to rc.d/sa-spamd makes it work.

@jrmarino
Copy link
Contributor

jrmarino commented Jan 5, 2015

Thanks, I made the change at the ports level and fixed some other ports with interpreters of /usr/bin/perl at the same time.

@jrmarino jrmarino closed this as completed Jan 5, 2015
@jrmarino
Copy link
Contributor

jrmarino commented Jan 6, 2015

pavalos, can you review this PR? Allegedly, this change broke FreeBSD and is "wrong". I find that a bit hard to believe myself, but what do you think?
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=196517

@pavalos
Copy link
Author

pavalos commented Jan 6, 2015

Interesting that behavior on FreeBSD is the opposite (what we are trying to fix) of ours. Without command_interpreter, we can't stop or restart spamd. I'm about to hop on a flight, but I'll try to see where the incompatibility is (gotta be rc.subr) after i land.

On Jan 5, 2015, at 4:25 PM, jrmarino notifications@github.com wrote:

pavalos, can you review this PR? Allegedly, this change broke FreeBSD and is "wrong". I find that a bit hard to believe myself, but what do you think?
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=196517


Reply to this email directly or view it on GitHub.

@pavalos
Copy link
Author

pavalos commented Jan 6, 2015

There's no rush. John is going to back out the change in the meantime.

Adam

Adam Weinberger
adamw@adamw.org
http://www.adamw.org

On 5 Jan, 2015, at 17:39, Peter Avalos peter@theshell.com wrote:

Interesting that behavior on FreeBSD is the opposite (what we are trying to fix) of ours. Without command_interpreter, we can't stop or restart spamd. I'm about to hop on a flight, but I'll try to see where the incompatibility is (gotta be rc.subr) after i land.

On Jan 5, 2015, at 4:25 PM, jrmarino notifications@github.com wrote:

pavalos, can you review this PR? Allegedly, this change broke FreeBSD and is "wrong". I find that a bit hard to believe myself, but what do you think?
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=196517


Reply to this email directly or view it on GitHub.

@jrmarino
Copy link
Contributor

pavalos, have you had a chance to review both dragonfly and freebsd's handling of command_interpreter? I backed out your changes upon Adam's request, but the idea is that we figure out where the problem is and submit a change request if necessary. It seems dragonfly is using command_interpreter correctly, so it could be a bug in FreeBSD

@jrmarino jrmarino reopened this Jan 20, 2015
@pavalos
Copy link
Author

pavalos commented Jan 21, 2015

On Jan 20, 2015, at 2:21 AM, jrmarino notifications@github.com wrote:

pavalos, have you had a chance to review both dragonfly and freebsd's handling of command_interpreter? I backed out your changes upon Adam's request, but the idea is that we figure out where the problem is and submit a change request if necessary. It seems dragonfly is using command_interpreter correctly, so it could be a bug in FreeBSD


Reply to this email directly or view it on GitHub #129 (comment).

I’m convinced command_interpreter is correct, but I’ve narrowed this down to FreeBSD’s ps behavior.

DragonFly:

ps -o "pid,command" -p 782825

PID COMMAND
782825 perl: /usr/local/bin/spamd -l --socketpath=/var/run/spamd/spamd.sock -d -r /var/run/spamd/spamd.pid (perl)

FreeBSD:
PID COMMAND
805 /usr/local/bin/spamd -c -d -r /var/run/spamd/spamd.pid (perl)

(Note the lack of “perl: “ in FreeBSD’s output) So this is why FreeBSD is failing with command_interpreter set, but I haven’t figured out why FreeBSD does this…

—Peter

@jrmarino
Copy link
Contributor

I thought you were on an email that explained the "perl" gets lost when spamd gets daemonized, at least on FreeBSD

@jrmarino
Copy link
Contributor

(but obviously not)

@pavalos
Copy link
Author

pavalos commented Jan 21, 2015

Well I just looked at the FreeBSD PR, and there’s some discussion there that I wasn't tracking on…So I agree that /usr/local/bin/spamd rewrites its command title in the daemonize routine on line 2929, but that still doesn’t explain why the interpreter (perl: ) part goes away.

That line in /usr/local/bin/spamd makes it go from:

/usr/local/bin/perl -T -w /usr/local/bin/spamd -c -d -r /var/run/spamd/spamd.pid

to:
/usr/local/bin/spamd -c -d -r /var/run/spamd/spamd.pid

But note that FreeBSD doesn’t have the interpreter listed (as “perl: “), regardless of that line in spamd. But I don’t know why...=

@jrmarino
Copy link
Contributor

You said that "command interpreter" is working on FreeBSD, but then you describe the "ps" mechanism that prevents it from working due to the behavior of ps.

This is why I was thinking there is actually a bug in FreeBSD. It's not working as advertised. It actually breaks if command_interpreter is specified.

@pavalos
Copy link
Author

pavalos commented Jan 21, 2015

Looking at it from another point of view…Why does our ps include “perl: “?

Not all daemonized perl programs do that…Look at munin-node on DragonFly:
/usr/local/bin/perl -wT /usr/local/sbin/munin-node

—Peter=

@jrmarino
Copy link
Contributor

You tell me. :)
All I know is that FreeBSD and DragonFly are not compatible in this aspect and DragonFly is the one that is operating as expected (I think). So we identified the problem but the analysis is ongoing. I'm just standing by for your recommendation and I'll try to help fix the problem based on that.

@pavalos
Copy link
Author

pavalos commented Jan 21, 2015

After some consultation with Sascha Wildner, he pointed me to this gem in perl’s mg.c:

#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

DragonFly’s setproctitle() has the “-“ capability, so if we were to patch perl, then the ps output will not have perl in it, and command_interpreter will not be necessary. What do you think?

@jrmarino
Copy link
Contributor

pro: it would make us more compatible with ports
con 1: It would still break release 4.0 and 3.8
con 2: It would make DFLY different from netbsd and openbsd. Right now freebsd is the odd one out.

requirement if we don't do anything: multiple dragonfly-only patches to "fix" ports.

The answer is not clear cut.

@pavalos
Copy link
Author

pavalos commented Jan 21, 2015

Well if DragonFly maintains setproctitle’s “-“ capability and perl wants to use it, then we should go in that direction (by patching perl source by adding a || defined(__DragonFly) to that section of mg.c.

As far as handling non-patched versions of perl, then we have command_interpreter…It’s not great, but I’m not sure what else to do…

@jrmarino
Copy link
Contributor

Okay, I patched all 3 version of perl.
I assume this resolves the issue, so I'm closing this ticket. Feel free to open it again if I'm wrong about that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants