Re: idle_in_transaction_timeout - Mailing list pgsql-hackers

On 2014-06-29 19:13:55 -0400, Tom Lane wrote:
> Andres Freund <andres@2ndquadrant.com> writes:
> > On 2014-06-29 12:53:56 -0400, Tom Lane wrote:
> >> I do not think it is: specifically, the notion
> >> that we will call ereport(FATAL) directly from a signal handler
> >> does not fill me with warm fuzzies.
> 
> > Aren't we already pretty much doing that for
> > SIGTERM/pg_terminate_backend() and recovery conflict interrupts?
> 
> We do that *only at safe spots*, ie CHECK_FOR_INTERRUPTS. Or at least
> I sure hope so. Otherwise interrupting, eg, malloc will lead to much
> unhappiness.

I was specifically thinking about us immediately reacting to those while
we're reading from the client. We're indeed doing that directly:

#1  0x000000000076648a in proc_exit (code=1) at /home/andres/src/postgresql/src/backend/storage/ipc/ipc.c:143
#2  0x00000000008bcbf7 in errfinish (dummy=0) at /home/andres/src/postgresql/src/backend/utils/error/elog.c:555
#3  0x00000000007909f7 in ProcessInterrupts () at /home/andres/src/postgresql/src/backend/tcop/postgres.c:2869
#4  0x0000000000790469 in die (postgres_signal_arg=15) at /home/andres/src/postgresql/src/backend/tcop/postgres.c:2604
#5  <signal handler called>
#6  0x00007fb7c8ca0ebb in __libc_recv (fd=10, buf=0xd5f240 <PqRecvBuffer>, n=8192, flags=-1) at
../sysdeps/unix/sysv/linux/x86_64/recv.c:29
#7  0x000000000066a07c in secure_read (port=0x1a29d30, ptr=0xd5f240 <PqRecvBuffer>, len=8192)   at
/home/andres/src/postgresql/src/backend/libpq/be-secure.c:317
#8  0x00000000006770b5 in pq_recvbuf () at /home/andres/src/postgresql/src/backend/libpq/pqcomm.c:854
#9  0x000000000067714f in pq_getbyte () at /home/andres/src/postgresql/src/backend/libpq/pqcomm.c:895
#10 0x000000000078d26b in SocketBackend (inBuf=0x7fffbc02bc30) at
/home/andres/src/postgresql/src/backend/tcop/postgres.c:335
#11 0x000000000078d659 in ReadCommand (inBuf=0x7fffbc02bc30) at
/home/andres/src/postgresql/src/backend/tcop/postgres.c:483

Note that we're *inside* recv() here. Both paths to recv, without ssl
and with, have called prepare_for_client_read() which sets
ImmediateInterruptOK = true. Right now I fail to see why it's safe to do
so, at least when inside openssl.

> > BUT: why is what ProcessInterrupts() is doing safe? Shouldn't it always
> > at least set whereToSendOutput = DestNone when FATALing while reading
> > (potentially via openssl)?
> 
> Uh, no, that would pretty much destroy the point of trying to report
> an error message at all.

I only mean that we should do so in scenarios where we're currently
reading from the client. For die(), while we're reading from the client,
we're sending a message the client doesn't expect - and thus
unsurprisingly doesn't report. The client will just report 'server
closed the connection unexpectedly'.

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services