Re: pgsql/src backend/tcop/postgres.c include/misc ...

Tom Lane wrote:
>
> "Hiroshi Inoue" <Inoue@tpf.co.jp> writes:
>
> > Before 7.1 QueryCancel flag was checked at the points
> > CHECK_FOR_INTERRUPTS are currently placed.
> > But the QueryCancel flag had nothing to do with die
> > interrupts.
>
> Indeed, and before 7.1 killing a backend with SIGTERM at a random time
> was horribly dangerous.  I did a bunch of retail patching at one point:
>
> 2001-01-12 16:53  tgl
>
>         * src/: backend/access/heap/heapam.c,
>         backend/access/nbtree/nbtinsert.c, backend/access/nbtree/nbtpage.c,
>         backend/access/transam/xact.c, backend/access/transam/xlog.c,
>         backend/commands/sequence.c, backend/commands/vacuum.c,
>         backend/storage/buffer/bufmgr.c, backend/storage/file/fd.c,
>         backend/storage/ipc/spin.c, backend/storage/lmgr/proc.c,
>         backend/tcop/postgres.c, backend/utils/cache/temprel.c,
>         backend/utils/init/postinit.c, backend/utils/mmgr/aset.c,
>         include/access/xlog.h, include/utils/elog.h: Add more
>         critical-section calls: all code sections that hold spinlocks are
>         now critical sections, so as to ensure die() won't interrupt us
>         while we are munging shared-memory data structures.  Avoid insecure
>         intermediate states in some code that proc_exit will call, like
>         palloc/pfree.  Rename START/END_CRIT_CODE to
>         START/END_CRIT_SECTION, since that seems to be what people tend to
>         call them anyway, and make them be called with () like a function
>         call, in hopes of not confusing pg_indent.  I doubt that this is
>         sufficient to make SIGTERM safe anywhere; there's just too much
>         code that could get invoked during proc_exit().
>
> and then gave up and proposed the current scheme.

I think the much more significant change is the following
one not the above one. ImmediateInterruptOK flag was
introduced and the flag is set to false except when the
backends are idle. I must have checked and objected to
the change then.

regards,
Hiroshi Inoue


CVSROOT:    /home/projects/pgsql/cvsroot
Module name:    pgsql
Changes by:    tgl@hub.org    01/01/14 00:08:17

Modified files:
    src/backend/access/nbtree: nbtinsert.c
    src/backend/access/transam: xact.c xlog.c
    src/backend/bootstrap: bootstrap.c
    src/backend/commands: vacuum.c analyze.c copy.c
    src/backend/executor: execProcnode.c
    src/backend/storage/buffer: bufmgr.c s_lock.c
    src/backend/storage/ipc: ipc.c spin.c
    src/backend/storage/lmgr: lock.c proc.c
    src/backend/tcop: postgres.c
    src/backend/utils/error: elog.c
    src/backend/utils/init: globals.c
    src/include/access: xlog.h
    src/include    : miscadmin.h
    src/include/storage: proc.h ipc.h
    src/include/tcop: tcopprot.h
    src/include/utils: elog.h
    src/interfaces/ecpg/preproc: pgc.l

Log message:
    Restructure backend SIGINT/SIGTERM handling so that 'die' interrupts
    are treated more like 'cancel' interrupts: the signal handler sets a
    flag that is examined at well-defined spots, rather than trying to cope
    with an interrupt that might happen anywhere.  See pghackers discussion
    of 1/12/01.