2010/2/15 Fujii Masao <masao.fu...@gmail.com>: > On Sun, Feb 14, 2010 at 11:52 PM, Magnus Hagander <mag...@hagander.net> wrote: >> 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?
Yes. >> 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. I changed your patch to this, because I find it a lot simpler. The change is in the checking in pgwin32_recv - there is no need to ever call waitforsinglesocket, we can just exit out early. Do you see any issue with that? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
pq_getbyte_if_available_on_win32_magnus.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers