Simplify sleeping while reading/writing from client
Looking again at the code after Andres' interrupt-handling patch series,
I got confused by the fact that there are several wait-retry loops in
different layers, and reading and writing works slightly differently.
I propose the attached, which pulls all the wait-retry logic up to
secure_read() and secure_write(). This makes the code a lot more
understandable.
- Heikki
Attachments:
0001-Simplify-waiting-logic-in-reading-from-writing-to-cl.patchapplication/x-patch; name=0001-Simplify-waiting-logic-in-reading-from-writing-to-cl.patchDownload+79-153
On Thu, Feb 5, 2015 at 6:45 PM, Heikki Linnakangas wrote:
Looking again at the code after Andres' interrupt-handling patch series, I
got confused by the fact that there are several wait-retry loops in
different layers, and reading and writing works slightly differently.I propose the attached, which pulls all the wait-retry logic up to
secure_read() and secure_write(). This makes the code a lot more
understandable.
Are you sure that it is a good idea to move the check of port->noblock
out of be_tls_[read|write] to an upper level? ISTM that we should set
errno and n to respectively EWOULDBLOCK and -1 in be_tls_[write|read]
when port->noblock and do nothing otherwise. In your patch those
values are set even if the port is in block mode.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 02/06/2015 10:38 AM, Michael Paquier wrote:
On Thu, Feb 5, 2015 at 6:45 PM, Heikki Linnakangas wrote:
Looking again at the code after Andres' interrupt-handling patch series, I
got confused by the fact that there are several wait-retry loops in
different layers, and reading and writing works slightly differently.I propose the attached, which pulls all the wait-retry logic up to
secure_read() and secure_write(). This makes the code a lot more
understandable.Are you sure that it is a good idea to move the check of port->noblock
out of be_tls_[read|write] to an upper level? ISTM that we should set
errno and n to respectively EWOULDBLOCK and -1 in be_tls_[write|read]
when port->noblock and do nothing otherwise. In your patch those
values are set even if the port is in block mode.
It simplifies the code to do all the sleeping and interrupt handling
code in the upper level, in secure_[read|write]. Do you see a problem
with it?
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2015-02-05 16:45:50 +0200, Heikki Linnakangas wrote:
Looking again at the code after Andres' interrupt-handling patch series, I
got confused by the fact that there are several wait-retry loops in
different layers, and reading and writing works slightly differently.
They don't really work all that differently?
I propose the attached, which pulls all the wait-retry logic up to
secure_read() and secure_write(). This makes the code a lot more
understandable.
Generally a good idea. Especially if we get more ssl implementations.
But I think it'll make the already broken renegotiation handling even
worse. Because now we're always entering do_handshake in nonblocking
mode (essentially).
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Feb 6, 2015 at 1:46 PM, Heikki Linnakangas wrote:
It simplifies the code to do all the sleeping and interrupt handling code in
the upper level, in secure_[read|write]. Do you see a problem with it?
Not directly. Reading the code I got uneasy with the fact that we fact
unconditionally those parameters even if port is in block mode.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 02/06/2015 11:50 AM, Andres Freund wrote:
On 2015-02-05 16:45:50 +0200, Heikki Linnakangas wrote:
I propose the attached, which pulls all the wait-retry logic up to
secure_read() and secure_write(). This makes the code a lot more
understandable.Generally a good idea. Especially if we get more ssl implementations.
But I think it'll make the already broken renegotiation handling even
worse. Because now we're always entering do_handshake in nonblocking
mode (essentially).
Good point. In the other thread, we concluded that the
SSL_do_handshake() call isn't really needed anyway, so attached are two
patches: 1. remove the SSL_do_handshake() calls, and 2. the previous
patch, to simplify the waiting logic.
- Heikki