Обсуждение: Signals in a BGW

I'm curious about handling signals in a background worker.

As I understand it, these are blocked when the BGW is born, until
enabled with BackgroundWorkerUnblockSignals() after possible
customization.

Is there a known, default behavior that some signals will
have, if I simply BackgroundWorkerUnblockSignals() without customizing?
Will SIGINT and SIGTERM, for example, have default handlers that
interact with the volatile flags in miscadmin.h that are used by
CHECK_FOR_INTERRUPTS, and set the process latch?

I notice that the worker_spi example code customizes SIGHUP
and SIGTERM, giving them handlers that set a (different, local
to the module) volatile flag, and set the process latch.

The example code does call CHECK_FOR_INTERRUPTS, though I assume
this won't detect ProcDie at all, now that the custom SIGTERM
handler is setting a different, local flag. Perhaps it still
does something useful with QueryCancel?

Does this use of a custom SIGTERM handler, setting a custom flag,
setting the process latch, and then checked in custom code,
accomplish something important that would not be accomplished
by a generic handler and CHECK_FOR_INTERRUPTS ?

I'm just not clearly grasping the reason it's customized here.

-Chap

On Tue, Dec 5, 2017 at 10:03 AM, Craig Ringer <craig@2ndquadrant.com> wrote:
> pglogical sets up its own handler 'handle_sigterm'. However, it now does
> much the same as src/backend/tcop/postgres.c's 'die' function, just without
> the single-user mode checks.

Documentation shows a simple example of that:
https://www.postgresql.org/docs/devel/static/source-conventions.html

> IMO it's silly to customise them, and a bad example.

I don't agree. It is not silly to have background workers being able
to take clean up actions and have their own tracking with some
sig_atomic_t flags.
-- 
Michael

On 12/04/2017 08:03 PM, Craig Ringer wrote:

> pglogical sets up its own handler 'handle_sigterm'. However, it now does
> much the same as src/backend/tcop/postgres.c's 'die' function. ...
> We used to have our own signal> handling but it gets seriously messy when you're calling into arbitrary
> PostgreSQL code that expects CHECK_FOR_INTERRUPTS() to work.
> 
> ...
> Personally I'd rather change the default bgw handler to 'die', make the
> sample bgworkers use CHECK_FOR_INTERRUPTS() properly, and be done

Short of reaching consensus to change the default bgw handler to 'die',
am I right to think any bgw that wanted to could set its own SIGTERM
handler to 'die' (its default SIGINT handler already being the normal
StatementCancelHandler), and use CHECK_FOR_INTERRUPTS(), and be cool?

> The default handler is bgworker_die ; see src/backend/postmaster
> /bgworker.c
> . I don't know if elog() is safe in a signal handler, but I guess in
> the absence of non-reentrant errcontext functions...

That does seem bold, doesn't it? I see there's a direct ereport(ERROR
in the standard FloatExceptionHandler also. Does that get exercised
much? I tried a quick select '1.0'::float8 / '0.0'::float8; but got
a more-specific 22012 division by zero, so it looks like such things
are mostly checked early and SIGFPE should be rare.

-Chap

On Wed, Dec 6, 2017 at 6:59 PM, Chapman Flack <chap@anastigmatix.net> wrote:
>> The default handler is bgworker_die ; see src/backend/postmaster
>> /bgworker.c
>> . I don't know if elog() is safe in a signal handler, but I guess in
>> the absence of non-reentrant errcontext functions...
>
> That does seem bold, doesn't it?

Yes, I think it's flat busted.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 2017-12-07 12:35:07 -0500, Robert Haas wrote:
> On Wed, Dec 6, 2017 at 6:59 PM, Chapman Flack <chap@anastigmatix.net> wrote:
> >> The default handler is bgworker_die ; see src/backend/postmaster
> >> /bgworker.c
> >> . I don't know if elog() is safe in a signal handler, but I guess in
> >> the absence of non-reentrant errcontext functions...
> >
> > That does seem bold, doesn't it?
> 
> Yes, I think it's flat busted.

We've had that discussion a couple times. The concensus so far has been
that FATALing is considered kinda ok, everything else not. But it
definitely has caused issues in the ast, mostly due to malloc being
entered while already in malloc.

Greetings,

Andres Freund

On 12/07/2017 02:58 PM, Andres Freund wrote:
> On 2017-12-07 12:35:07 -0500, Robert Haas wrote:
>> On Wed, Dec 6, 2017 at 6:59 PM, Chapman Flack <chap@anastigmatix.net> wrote:
>>>> The default handler is bgworker_die ; see src/backend/postmaster
>>>> /bgworker.c
>>>> . I don't know if elog() is safe in a signal handler, but I guess in
>>>> the absence of non-reentrant errcontext functions...
>>>
>>> That does seem bold, doesn't it?
>>
>> Yes, I think it's flat busted.
> 
> We've had that discussion a couple times. The concensus so far has been
> that FATALing is considered kinda ok, everything else not. But it
> definitely has caused issues in the ast, mostly due to malloc being
> entered while already in malloc.

Well, bgworker.c:bgworker_die() FATALs, so that might be kinda ok,
but postgres.c:FloatExceptionHandler() ERRORs, so what's up with that?

Has it just not caused much trouble because the existing math
operators test their operands rather than relying on exceptions,
and it might only get called in case of some extension that did
float operations without checking?

I admit I've been trying to think of a better thing it could do,
and it seems challenging ... ideally you'd want an ERROR to happen,
and soon (before trying to evaluate more of the expression), from
code that might not be checking right away ... but how could that
be made to work?

-Chap