Thread: Idle transaction cancel/timeout and SSL revisited

Hello Hackers,

After reading up through archives on the two $subj related TODO items
I'm under impression that the patches[1,2] didn't make it mainly because
of the risk of breaking SSL internals if we try to longjump out of the
signal handler in the middle of a blocking SSL read and/or if we try to
report that final "transaction/connection aborted" notice to the client.

What if instead of trying to escape from the signal handler we would set
a flag and use it to simulate EOF after the recv() is restarted due to
EINTR?  From the backend perspective it should look like the client has
just hang up.

Both SSL and cleartext connections are routed through secure_raw_read,
so it seems like a good candidate for checking the flag we would set in
StatementCancelHandler().  (Should we also add the same check in
interactive_getc()?)

Ultimately, in PostgresMain() the ReadCommand() would return EOF and we
can either let the existing code shutdown the backend, or add special
handling that will only abort the transaction and report the case to the
client, which was the intent in[1].  And if that works out, the timeout
handler in[2] can simply reuse the flag.

A potential problem with this is that SSL might perform some eager
cleanup when seeing EOF, so the backend might need to reset/restart the
connection (is that possible?)

I'm attaching a patch (that doesn't compile yet :-p) in hope it can be
useful for the purpose of the discussion.  Notably, secure_raw_read()
can't know about IdleTransactionCancelPending and I'm not sure what
would be the best way to let it see that (is it too much of a shortcut
anyway?)

--
Alex

[1] http://www.postgresql.org/message-id/1262268211.19367.10736.camel@ebony
[2] http://www.postgresql.org/message-id/538DC843.2070608@dalibo.com

Hi,

On 2014-11-15 00:11:36 +0300, Alex Shulgin wrote: 
> After reading up through archives on the two $subj related TODO items
> I'm under impression that the patches[1,2] didn't make it mainly because
> of the risk of breaking SSL internals if we try to longjump out of the
> signal handler in the middle of a blocking SSL read and/or if we try to
> report that final "transaction/connection aborted" notice to the client.

I've written a patch that allows that - check
http://archives.postgresql.org/message-id/20140927225421.GE5423%40alap3.anarazel.de

I feel pretty confident that that's the way to go. I just need some time
to polish it.

> What if instead of trying to escape from the signal handler we would set
> a flag and use it to simulate EOF after the recv() is restarted due to
> EINTR?  From the backend perspective it should look like the client has
> just hang up.

That's pretty much what the above does. Except that it actually deals
with blocking syscalls by not using them and relying on latches/select
instead.

Greetings,

Andres Freund

Andres Freund <andres@2ndquadrant.com> writes:
>
> On 2014-11-15 00:11:36 +0300, Alex Shulgin wrote: 
>> After reading up through archives on the two $subj related TODO items
>> I'm under impression that the patches[1,2] didn't make it mainly because
>> of the risk of breaking SSL internals if we try to longjump out of the
>> signal handler in the middle of a blocking SSL read and/or if we try to
>> report that final "transaction/connection aborted" notice to the client.
>
> I've written a patch that allows that - check
> http://archives.postgresql.org/message-id/20140927225421.GE5423%40alap3.anarazel.de
>
> I feel pretty confident that that's the way to go. I just need some time
> to polish it.
>
>> What if instead of trying to escape from the signal handler we would set
>> a flag and use it to simulate EOF after the recv() is restarted due to
>> EINTR?  From the backend perspective it should look like the client has
>> just hang up.
>
> That's pretty much what the above does. Except that it actually deals
> with blocking syscalls by not using them and relying on latches/select
> instead.

Neat, I must have missed it.

--
Alex

Andres Freund <andres@2ndquadrant.com> writes:
> On 2014-11-15 00:11:36 +0300, Alex Shulgin wrote: 
>> After reading up through archives on the two $subj related TODO items
>> I'm under impression that the patches[1,2] didn't make it mainly because
>> of the risk of breaking SSL internals if we try to longjump out of the
>> signal handler in the middle of a blocking SSL read and/or if we try to
>> report that final "transaction/connection aborted" notice to the client.
>
> I've written a patch that allows that - check
> http://archives.postgresql.org/message-id/20140927225421.GE5423%40alap3.anarazel.de
>
> I feel pretty confident that that's the way to go. I just need some time
> to polish it.
>
>> What if instead of trying to escape from the signal handler we would set
>> a flag and use it to simulate EOF after the recv() is restarted due to
>> EINTR?  From the backend perspective it should look like the client has
>> just hang up.
>
> That's pretty much what the above does. Except that it actually deals
> with blocking syscalls by not using them and relying on latches/select
> instead.

Yay, that's nice, because my EOF approach is sort of fishy.

I've checked the patches: they apply and compile on top of current HEAD
(one failed hunk in pqcomm.c), so I can help with testing if needed.

There is a (short) list of items that caught my attention.  I will post
in reply to your patches thread.

--
Alex