[systemd-devel] [PATCH] login: share VT-signal handler between sessions

[Lennart Poettering]

On Mon, 11.08.14 18:21, David Herrmann (dh.herrmann at gmail.com) wrote:

> +        /*
> +         * SIGRTMIN is used as global VT-release signal, SIGRTMIN + 1 is used
> +         * as VT-acquire signal. We ignore any acquire-events (yes, we still
> +         * have to provide a valid signal-number for it!) and acknowledge all
> +         * release events immediately.
> +         */
> +
> +        if (SIGRTMIN + 1 > SIGRTMAX) {
> +                log_error("Not enough real-time signals available: %u-%u", SIGRTMIN, SIGRTMAX);
> +                return -EINVAL;
> +        }

POSIX says at least 20 or so rtsigs need to be available. Either way, I
am pretty sure this shouldn't be a "soft" runtime check. I'd just
replace this with assert_se() or maybe assert().

> +
> +        r = sigprocmask_many(SIG_BLOCK, SIGRTMIN, SIGRTMIN + 1, -1);

I really don't see how in real-life this could ever fail. I think I'd
simplify this by enclosing this in assert_se(... >= 0) or so. After all,
this just flips bits, doesn't allocate any resources...

> +        if (r < 0) {
> +                log_error("Cannot block SIGRTMIN and SIGRTMIN + 1: %s", strerror(-r));
> +                return r;
> +        }
> +
> +        r = sd_event_add_signal(m->event, NULL, SIGRTMIN, manager_vt_switch, m);
> +        if (r < 0)
> +                return r;

With this code you block, but do not ignore SGRTMIN+1. Now, rtsigs
actually are implemented in a queue, multiple instances of the same
signal might be queued up. If you simply block dispatching, then the
queue will eventually overrun blocking all other signals... Hence, I
think there needs to be a dummy signal handler assigned to SIGRTMIN+1
here, so that the signals are dequeued...

(Thinking about it, it might make sense to export a dummy signal handler
from sd-event which people can just use here...)

Lennart

-- 
Lennart Poettering, Red Hat