On Sun, Feb 14, 2010 at 11:52 PM, Magnus Hagander <[email protected]> wrote:
> Sorry about the delay in responding to this.
Thanks for the response.
> Remember that the win32 code *always* puts the socket in non-blocking
> mode. So we can't just "teach the layer about it". We need some way to
> pass the information down that this is actually something we want to
> be non-blocking, and it can't be the normal flag on the socket. I
> don't really have an idea of where else we'd put it though :( It's in
> the port structure, but not beyond it.
Right.
BTW, pq_getbyte_if_available() always changes the socket to non-blocking
and blocking mode before and after calling secure_read(), respectively.
This code seems wrong on win32. Because, as you said, the socket is always
in non-blocking mode on win32. We should change pq_getbyte_if_available()
so as not to change the socket mode only in win32?
> What we could do, is have an ugly global flag specifically for the
> use-case we have here. Assuming we do create a plataform specific
> pq_getbyte_if_available(), the code-path that would have trouble now
> would be when we call pq_getbyte_if_available(), and it in turns asks
> the socket if there is data, there is, but we end up calling back into
> the SSL code to fetch the data, and it gets an incomplete packet.
> Correct? So the path is basically:
>
> pq_getbyte_if_available() -> secure_read() -> SSL_read() ->
> my_sock_read() -> pgwin32_recv()
>
> Given that we know we are working on a single socket here, we could
> use a global flag to tell pgwin32_recv() to become nonblocking. We
> could set this flag directly in the win32-specific version of
> pq_getbyte_if_available(), and make sure it's cleared as soon as we
> exit.
>
> It will obviously fail if we do anything on a *different* socket
> during this time, so it has to be set for a very short time. But that
> seems doable. And we don't call any socket stuff from signal handlers
> so that shouldn't cause issues.
Agreed. Here is the patch which does that (including the above-mentioned
change). I haven't tested it yet because I failed in creating the build
environment for the MSVC :( I'll try to create that again, and test it.
Though I'm not sure how long it takes.
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
*** a/src/backend/libpq/pqcomm.c
--- b/src/backend/libpq/pqcomm.c
***************
*** 837,845 **** pq_getbyte_if_available(unsigned char *c)
--- 837,849 ----
}
/* Temporarily put the socket into non-blocking mode */
+ #ifdef WIN32
+ pgwin32_noblock = 1;
+ #else
if (!pg_set_noblock(MyProcPort->sock))
ereport(ERROR,
(errmsg("couldn't put socket to non-blocking mode: %m")));
+ #endif
MyProcPort->noblock = true;
PG_TRY();
{
***************
*** 851,866 **** pq_getbyte_if_available(unsigned char *c)
--- 855,878 ----
* The rest of the backend code assumes the socket is in blocking
* mode, so treat failure as FATAL.
*/
+ #ifdef WIN32
+ pgwin32_noblock = 0;
+ #else
if (!pg_set_block(MyProcPort->sock))
ereport(FATAL,
(errmsg("couldn't put socket to blocking mode: %m")));
+ #endif
MyProcPort->noblock = false;
PG_RE_THROW();
}
PG_END_TRY();
+ #ifdef WIN32
+ pgwin32_noblock = 0;
+ #else
if (!pg_set_block(MyProcPort->sock))
ereport(FATAL,
(errmsg("couldn't put socket to blocking mode: %m")));
+ #endif
MyProcPort->noblock = false;
return r;
*** a/src/backend/port/win32/socket.c
--- b/src/backend/port/win32/socket.c
***************
*** 13,18 ****
--- 13,26 ----
#include "postgres.h"
+ /*
+ * This indicates whether pgwin32_recv() is blocked until the receive
+ * operation has been finished. A value of zero blocks it even if
+ * the socket is set to non-blocking mode. A non-zero value makes it
+ * return immediately whether data is available or not.
+ */
+ int pgwin32_noblock = 0;
+
#undef socket
#undef accept
#undef connect
***************
*** 315,321 **** pgwin32_recv(SOCKET s, char *buf, int len, int f)
for (n = 0; n < 5; n++)
{
if (pgwin32_waitforsinglesocket(s, FD_READ | FD_CLOSE | FD_ACCEPT,
! INFINITE) == 0)
return -1; /* errno already set */
r = WSARecv(s, &wbuf, 1, &b, &flags, NULL, NULL);
--- 323,330 ----
for (n = 0; n < 5; n++)
{
if (pgwin32_waitforsinglesocket(s, FD_READ | FD_CLOSE | FD_ACCEPT,
! (pgwin32_noblock == 0) ? INFINITE : 0)
! == 0)
return -1; /* errno already set */
r = WSARecv(s, &wbuf, 1, &b, &flags, NULL, NULL);
*** a/src/include/port/win32.h
--- b/src/include/port/win32.h
***************
*** 283,288 **** int pgwin32_send(SOCKET s, char *buf, int len, int flags);
--- 283,290 ----
const char *pgwin32_socket_strerror(int err);
int pgwin32_waitforsinglesocket(SOCKET s, int what, int timeout);
+ extern int pgwin32_noblock;
+
/* in backend/port/win32/security.c */
extern int pgwin32_is_admin(void);
extern int pgwin32_is_service(void);
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
