PATCH SSL_pending() checks in libpq/fe-misc.c
Hello:
I took a look at the SSL code in libpq/fe-misc.c and noticed what I
think is a small problem. A patch is included at the bottom of this
email against anoncvs TopOfTree this evening.
The SSL library buffers input data internally. Nowhere in libpq's code
is this buffer being checked via SSL_pending(), which can lead to a
condition where once in a while a socket appears to "hang" or "lag".
This is because select() won't see bytes buffered by the library. A
condition like this is most likely to occur when the library's read
buffer has been filled previously and another read is to be performed.
If the end of the backend's transmission was less than one SSL frame
payload away from the last byte returned in the previous read, this will
likely hang. Trust me that I learned of this most painfully...
I am looking deeper at how to enable non-blocking SSL sockets in libpq.
As Tom Lane states, this is primarily a matter of checking SSL error
codes, particularly for SSL_WANT_READ and SSL_WANT_WRITE, and reacting
appropriately. I'll see about that as I have more free time.
Even though I'm doing this, I tend to agree with Tom that SSH tunnels
are a really good way to make the whole SSL problem just go away.
My quick patch to perform the SSL_pending() checks:
===================================================================
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-misc.c,v
retrieving revision 1.70
diff -r1.70 fe-misc.c
350a351
* -or- if SSL is enabled and used, is it buffering bytes?
361a363,371
/* Check for SSL library buffering read bytes */
#ifdef USE_SSL
if (conn->ssl && SSL_pending(conn->ssl) > 0)
{
/* short-circuit the select */
return 1;
}
#endif
784a795,797
* If SSL enabled and used and forRead, buffered bytes short-circuit the
* call to select().
*
801a815,823
/* Check for SSL library buffering read bytes */
#ifdef USE_SSL
if (forRead && conn->ssl && SSL_pending(conn->ssl) > 0)
{
/* short-circuit the select */
return 0;
}
#endif
_Of_course_ I am just fine with this patch being under a Berkeley-style
license and included in PostgreSQL.
Cheers.
--
Jack Bates
Portland, OR, USA
http://www.floatingdoghead.net
Got privacy?
My PGP key: http://www.floatingdoghead.net/pubkey.txt
Would you send over a context diff, diff -c?
---------------------------------------------------------------------------
Jack Bates wrote:
Hello:
I took a look at the SSL code in libpq/fe-misc.c and noticed what I
think is a small problem. A patch is included at the bottom of this
email against anoncvs TopOfTree this evening.The SSL library buffers input data internally. Nowhere in libpq's code
is this buffer being checked via SSL_pending(), which can lead to a
condition where once in a while a socket appears to "hang" or "lag".
This is because select() won't see bytes buffered by the library. A
condition like this is most likely to occur when the library's read
buffer has been filled previously and another read is to be performed.
If the end of the backend's transmission was less than one SSL frame
payload away from the last byte returned in the previous read, this will
likely hang. Trust me that I learned of this most painfully...I am looking deeper at how to enable non-blocking SSL sockets in libpq.
As Tom Lane states, this is primarily a matter of checking SSL error
codes, particularly for SSL_WANT_READ and SSL_WANT_WRITE, and reacting
appropriately. I'll see about that as I have more free time.Even though I'm doing this, I tend to agree with Tom that SSH tunnels
are a really good way to make the whole SSL problem just go away.My quick patch to perform the SSL_pending() checks:
===================================================================
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-misc.c,v
retrieving revision 1.70
diff -r1.70 fe-misc.c
350a351* -or- if SSL is enabled and used, is it buffering bytes?
361a363,371
/* Check for SSL library buffering read bytes */
#ifdef USE_SSL
if (conn->ssl && SSL_pending(conn->ssl) > 0)
{
/* short-circuit the select */
return 1;
}
#endif784a795,797
* If SSL enabled and used and forRead, buffered bytes short-circuit the
* call to select().
*801a815,823
/* Check for SSL library buffering read bytes */
#ifdef USE_SSL
if (forRead && conn->ssl && SSL_pending(conn->ssl) > 0)
{
/* short-circuit the select */
return 0;
}
#endif_Of_course_ I am just fine with this patch being under a Berkeley-style
license and included in PostgreSQL.Cheers.
--
Jack Bates
Portland, OR, USA
http://www.floatingdoghead.netGot privacy?
My PGP key: http://www.floatingdoghead.net/pubkey.txt---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026
Bruce:
I've attached the original source file that I modified, my modified
version, and the output from 'diff -c fe-misc.c fe-misc.c.jack'.
Lack of CVS tags makes this the best way for me to get this to you.
Let me know if you need anything else.
I am no longer pursuing a total non-blocking implementation. I haven't
found a good way to test it with the type of work that I do with
PostgreSQL. I do use blocking SSL sockets with this mod and have had no
problem whatsoever. The bug that I fixed in this patch is exceptionally
hard to reproduce reliably.
THANKS AGAIN!!!
--
Jack Bates
Portland, OR, USA
http://www.floatingdoghead.net
Got privacy?
My PGP key: http://www.floatingdoghead.net/pubkey.txt
Attachments:
diff-ctext/plain; charset=US-ASCII; name=diff-cDownload
*** fe-misc.c Sat May 4 00:11:02 2002
--- fe-misc.c.jack Wed May 1 23:17:08 2002
***************
*** 348,353 ****
--- 348,354 ----
/*
* pqReadReady: is select() saying the file is ready to read?
+ * JAB: -or- if SSL is enabled and used, is it buffering bytes?
* Returns -1 on failure, 0 if not ready, 1 if ready.
*/
int
***************
*** 359,364 ****
--- 360,374 ----
if (!conn || conn->sock < 0)
return -1;
+ /* JAB: Check for SSL library buffering read bytes */
+ #ifdef USE_SSL
+ if (conn->ssl && SSL_pending(conn->ssl) > 0)
+ {
+ /* short-circuit the select */
+ return 1;
+ }
+ #endif
+
retry1:
FD_ZERO(&input_mask);
FD_SET(conn->sock, &input_mask);
***************
*** 782,787 ****
--- 792,800 ----
/*
* pqWait: wait until we can read or write the connection socket
*
+ * JAB: If SSL enabled and used and forRead, buffered bytes short-circuit the
+ * call to select().
+ *
* We also stop waiting and return if the kernel flags an exception condition
* on the socket. The actual error condition will be detected and reported
* when the caller tries to read or write the socket.
***************
*** 800,805 ****
--- 813,827 ----
return EOF;
}
+ /* JAB: Check for SSL library buffering read bytes */
+ #ifdef USE_SSL
+ if (forRead && conn->ssl && SSL_pending(conn->ssl) > 0)
+ {
+ /* short-circuit the select */
+ return 0;
+ }
+ #endif
+
if (forRead || forWrite)
{
retry5:
Your patch has been added to the PostgreSQL unapplied patches list at:
http://candle.pha.pa.us/cgi-bin/pgpatches
I will try to apply it within the next 48 hours.
---------------------------------------------------------------------------
jack@floatingdoghead.net wrote:
Bruce:
I've attached the original source file that I modified, my modified
version, and the output from 'diff -c fe-misc.c fe-misc.c.jack'.Lack of CVS tags makes this the best way for me to get this to you.
Let me know if you need anything else.
I am no longer pursuing a total non-blocking implementation. I haven't
found a good way to test it with the type of work that I do with
PostgreSQL. I do use blocking SSL sockets with this mod and have had no
problem whatsoever. The bug that I fixed in this patch is exceptionally
hard to reproduce reliably.THANKS AGAIN!!!
--
Jack Bates
Portland, OR, USA
http://www.floatingdoghead.netGot privacy?
My PGP key: http://www.floatingdoghead.net/pubkey.txt
Content-Description:
[ Attachment, skipping... ]
Content-Description:
[ Attachment, skipping... ]
Content-Description:
[ Attachment, skipping... ]
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026