RPMSQ_DFL is like passing NULL to rpmsqSetAction(), this already
set the default action but now we have a nice name for it too.
RPMSQ_IGN is the only special case needed in rpmsqSetAction() as
it corresponds to an actual function. Return RPMSQ_ERR for any
signal not supported by rpmsq.
This doesn't actually change any core behavior as such.
SIGPIPE is business as usual when pipes are involved and require
no special warnings (that's why we're handling it after all), but
getting killed by other means seems still worth logging.
Programming errors like SIGSEGV and SIGBUS need to get through no
matter what and blocking them is undefined behavior anyway.
The odd man out in this list is SIGTSTP which is just otherwise useful
and not harmful since the process can be continued afterwards.
For rpm's purposes blocking all signals and restoring to previous
status is the only necessary operation, make this part of the
official API to make it available everywhere and replace rpmdb
internal signal handling code with it.
Noteworthy points:
- The block/unblock operation is reference counted so these can be
nested to arbitrary level without messing up things, much like
the internal chroot API.
- The rpmdb code used to remove rpmsq signals from the blocked mask, but
blocked should really mean blocked, really. The pending signals will be
delivered when unblocked and there's no need for us to mess with it.
- Unlike the rpmdb variant, we now Run rpmsqPoll() *after* unblocking
the signals. This actually matters now because rpmsqPoll() honors blocked
signals since commit 07620f4ae7.
This is more in line with how "normal" signals behave - if a signal
of the same type is already pending then the subsequent signals of
that type are dropped.
Queued signals might get processed much much later than they arrive,
and almost anything can happen in the meanwhile. Such as program
blocking signals before entering a critical piece of code, which
routinely calls rpmsqPoll() underneath since the code is not always
critical. Such as rpmdb iterator init/free - sometimes they're
called during harmless read-only query, at other times they're in
middle of transaction...
We always want the signal queue either enabled for the certain set
of signals or completely disabled, make the API reflect that and
make the switch "atomic", ie signal delivery is disabled while
changing state.
Perhaps more importantly, this allows changing signal handlers "offline",
so an application can set its own signal handler for, say, SIGINT
*in case* the signal queue is activated.
Copy the info contents on signal arrival, pass on to handlers during
poll round. There are some pointers in siginfo_t whose validity might
be questionable at the time we get it, but then those pointers like
address of segfault aren't exactly something to go poking at anyway.
The original rpmsq API is somehow backwards and upside down: it allows
overriding the handler when enabling the queue, but if the queue is
to be enabled for a signal, then the only possible handler for
it is the action that stores the signal in the queue. If you wanted
to have some other kind of behavior you wouldn't want to enable the
queue for that signal to begin with!
What applications need is the ability to override what happens at
signal poll, not arrival time - arrival time is at odds with the
whole queue notion. So introduce a poll function which runs the
handlers for caught signals, clearing them on the way. Add to that
the notion of default handler which is called if no custom handler
is set. Now this can be trivially used to replace the signal foobar
inside rpmdb and suddenly applications can replace the standard behavior
with a simple
void my_sigint_handler(....)
{
/* do foo */
}
rpmsqEnable(SIGINT, my_sigint_handler);
...and the custom hander will be called when a SIGINT has arrived
and the next signal polling round from rpmdb internals or whever
occurs.
rpmsqAction() is removed from the API/ABI here, but it's not as if
it ever was useful to anybody anyway.
Life's too short to keep worrying about ancient junk forever.
Even Hurd has this now, can you imagine? (Hurd was the reason
this was special-cased back in 2008)
There's little better way of tracking set of signals than a sigset_t
with its associated APIs. For one, this way we dont for example need to
loop through the table to see if a signal is active or not.
Note that This drops the "fancy" reference counting: calling rpmsqEnable()
with different handlers would increase the refcount but not actually change
the behavior, so the refcount means exactly what? The refcounting also
hasn't been used by rpm at all/in a long time, because whether its
active or not is tracked by the rpmdb code which is the only place
really knowing if its needed or not.
On Tue, Feb 17, 2015, at 07:07 AM, Florian Festi wrote:
> Sorry, for the last response. DevConf takes its toll...
>
> On 01/23/2015 04:07 AM, Colin Walters wrote:
> > Numerous consumers of librpm use it in a pattern where they're
> > constructing fresh chroots. For example, rpm-ostree operates this
> > way, and is used to provide atomic upgrades in concert with rpm.
> >
> > If the process dies due to SIGINT or another signal, the root can
> > simply be discarded.
> >
> > Currently today, rpm-ostree undoes the signal handlers after loading
> > librpm so that Control-C does what I want, but there's still a race
> > condition where the interrupt can be lost.
> >
> > Add an API so callers can disable the behavior.
>
> Is there any chance someone would want to switch them back on?
I can't think of one offhand...tools that interact with a live root
should be happy with what RPM does today, right?
> My gut
> feeling tells me this should rather be rpmsqSetInterruptSafety(int on);
But here's a patch which does it, in case you prefer it. I did write
a better API doc this time.
From ae6d2de85b7b81cf91318183ba253402ac538785 Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
Date: Thu, 22 Jan 2015 17:57:14 -0500
Subject: [PATCH] Add API to disable librpm's use of Unix signal handlers
Numerous consumers of librpm use it in a pattern where they're
constructing fresh chroots. For example, rpm-ostree operates this
way, and is used to provide atomic upgrades in concert with rpm.
If the process dies due to SIGINT or another signal, the root can
simply be discarded.
Currently today, rpm-ostree undoes the signal handlers after loading
librpm so that Control-C does what I want, but there's still a race
condition where the interrupt can be lost.
Add an API so callers can disable the behavior.
- Also remove additional thread protection: we're not supporting
threads anywhere else either. If/when thread-protection is added,
this is ulikely to be the first place anyway...
- use waitpid rather than SIGCHLD reaper.
- rip out DB_PRIVATE revert if not NPTL, it's not the right thing to do.
CVS patchset: 7761
CVS date: 2005/02/13 03:01:09
Panu Matilainen