On 07/16/2011 12:46 AM, Tom Lane wrote:
> I think the direction to move in ought to be to use the existing buffer
> as-is, and have pqCheckOutBufferSpace refuse to enlarge the buffer while
> we are in "write frozen" state. It should be OK to append data to the
> buffer, though, so long as we remember how much we're allowed to pass to
> SSL_write when we next try to write.
Alternative to freezing the outBuffer would be to set
SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER mode during SSL initialisation.
That would enable the buffer address to be changed in-between the
SSL_write calls, so long as the content remains the same. Attached
is a patch that uses the single buffer approach described by Tom, but
with a moving SSL write buffer enabled.
Modifying pqCheckOutBufferSpace is also an option, but it'd break some
(arguably already broken) client applications that don't have proper
retry handling. Notably some versions of psycopg2 have problems with
handling zero return values from PQputCopyData. So ISTM that from
backporting perspective the moving write buffer is a bit safer.
I'll see if I can come up with a test case for the SSL_read retry before
attempting to fix that.
regards,
Martin
diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 17dde4a..977e37c 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -793,20 +793,26 @@ pqSendSome(PGconn *conn, int len)
while (len > 0)
{
int sent;
+ int write_size = len;
char sebuf[256];
-#ifndef WIN32
- sent = pqsecure_write(conn, ptr, len);
-#else
-
+#ifdef WIN32
/*
* Windows can fail on large sends, per KB article Q201213. The
* failure-point appears to be different in different versions of
* Windows, but 64k should always be safe.
*/
- sent = pqsecure_write(conn, ptr, Min(len, 65536));
+ write_size = Min(len, 65536);
+#endif
+
+#ifdef USE_SSL
+ /* We have leftovers from previous SSL write, send these first. */
+ if (conn->ssl_retry_bytes)
+ write_size = conn->ssl_retry_bytes;
#endif
+ sent = pqsecure_write(conn, ptr, write_size);
+
if (sent < 0)
{
/*
@@ -857,11 +863,30 @@ pqSendSome(PGconn *conn, int len)
return -1;
}
}
+#ifdef USE_SSL
+ else if (sent == 0)
+ {
+ /*
+ * We are in non-blocking mode, set up for retrying a SSL write
+ * operation. OpenSSL requires the same buffer to be passed to the
+ * write retry, so we must remember the write size. However, we
+ * don't need to keep the original buffer address as the
+ * SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER mode ought to be set.
+ */
+ if (!conn->ssl_retry_bytes)
+ conn->ssl_retry_bytes = write_size;
+ }
+#endif
else
{
ptr += sent;
len -= sent;
remaining -= sent;
+#ifdef USE_SSL
+ /* Forget the retry buffer size if all the data has been sent */
+ if (conn->ssl_retry_bytes)
+ conn->ssl_retry_bytes = 0;
+#endif
}
if (len > 0)
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index cd1292c..540561d 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -757,6 +757,9 @@ init_ssl_system(PGconn *conn)
#endif
return -1;
}
+
+ /* Enable moving write buffer to simplify write retry handling. */
+ SSL_CTX_set_mode(SSL_context, SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER);
}
#ifdef ENABLE_THREAD_SAFETY
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index d56ef5d..ae212a6 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -407,6 +407,8 @@ struct pg_conn
X509 *peer; /* X509 cert of server */
char peer_dn[256 + 1]; /* peer distinguished name */
char peer_cn[SM_USER + 1]; /* peer common name */
+
+ int ssl_retry_bytes; /* buffer size for SSL write retry */
#ifdef USE_SSL_ENGINE
ENGINE *engine; /* SSL engine, if any */
#else
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
