Patch for select and APC on win32

Here's a patch implementing the "thread method" to workaround the bug
with socket calls in signal handlers. See details in mail to
pgsql-hackers-win32 a couple of minutes ago.

Looks ok, but wouldn't it be better placed in pgstat.c?

My $0.02,
Claudio

--- 
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these disclaimers and policies see 
<a
href="http://www.memetrics.com/emailpolicy.html">http://www.memetrics.com/em
ailpolicy.html</a>

Here's a patch implementing the "thread method" to

workaround the bug

with socket calls in signal handlers. See details in mail to
pgsql-hackers-win32 a couple of minutes ago.

Looks ok, but wouldn't it be better placed in pgstat.c?

Actually, I don't think so. I considered it, and chose to put it in
postmaster.c for the following reason:

The functon pgstat_beterm itself is *not* the problem. In theory, it can
be called from places that are not signal handlers (sure, it's not done
today I think, but internal-API-wise, it could). That goes against
putting the fix ther.

Also, by putting the code inside the actual signal handler function, we
put the fix close to the problem, which makes it more visible in the
"trouble area". The problem is, IMHO, in the signal handler, and not in
the actual pgstat function.

I'll be happy to change this if you still think it's better, but I
figured I shuold put out my argument for doing what I did first :-)

//Magnus

Here's a patch implementing the "thread method" to

workaround the bug

with socket calls in signal handlers. See details in mail to
pgsql-hackers-win32 a couple of minutes ago.

Looks ok, but wouldn't it be better placed in pgstat.c?

Actually, I don't think so. I considered it, and chose to put it in
postmaster.c for the following reason:

The functon pgstat_beterm itself is *not* the problem. In theory, it can
be called from places that are not signal handlers (sure, it's not done
today I think, but internal-API-wise, it could). That goes against
putting the fix ther.

Sure, like I said, my 2c. Just looks a little out of place. Understand point
on API, but think it is clear that this isn't a win32 replacement for
pgstat_beterm, but a win32 replacement for pgstat_beterm *called from a
signal handler* (perhaps a function name change would make it this clearer).

In any case, not fussed.

What I am wondering about now, is where else we need to change? AFAICS,
there is (at least?) one signal handler that performs sockets ops, namely
Async_NotifyHandler.

Cheers,
Claudio

--- 
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these disclaimers and policies see 
<a
href="http://www.memetrics.com/emailpolicy.html">http://www.memetrics.com/em
ailpolicy.html</a>

Here's a patch implementing the "thread method" to

workaround the bug

with socket calls in signal handlers. See details in mail to
pgsql-hackers-win32 a couple of minutes ago.

Looks ok, but wouldn't it be better placed in pgstat.c?

Actually, I don't think so. I considered it, and chose to put it in
postmaster.c for the following reason:

The functon pgstat_beterm itself is *not* the problem. In

theory, it can

be called from places that are not signal handlers (sure,

it's not done

today I think, but internal-API-wise, it could). That goes against
putting the fix ther.

Sure, like I said, my 2c. Just looks a little out of place.
Understand point
on API, but think it is clear that this isn't a win32 replacement for
pgstat_beterm, but a win32 replacement for pgstat_beterm *called from a
signal handler* (perhaps a function name change would make it
this clearer).

Could be. I think the comment makes it clear, though. But a function
rename is easy enough - have any suggestions?

What I am wondering about now, is where else we need to change? AFAICS,
there is (at least?) one signal handler that performs sockets
ops, namely
Async_NotifyHandler.

Actually, I don't think we need to do anything about that one. This
signal handler is used in the backend (not postmaster), and the backend
never calls selcet(). The other calls (recv, send etc) return correct
return values even when a socket call is made in the APC, I'm fairly
certain.

//Magnus

What I am wondering about now, is where else we need to change? AFAICS,
there is (at least?) one signal handler that performs sockets
ops, namely Async_NotifyHandler.

Actually, I don't think we need to do anything about that one. This
signal handler is used in the backend (not postmaster), and the backend
never calls selcet(). The other calls (recv, send etc) return correct
return values even when a socket call is made in the APC, I'm fairly
certain.

Not so sure. The reason I brought this up was because I tried the same test
(as I wrote before using select()) with recv(), and it returns -1 when
interrupted by an APC, which is ok, but errno is 0, and WSA/GetLastError
returns 997 (ERROR_IO_PENDING). [EINTR or WSAEINTR would have been nice].
Presumably send() also misbehaves.

Seems like it might actually be something to consider.

Cheers,
Claudio

--- 
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these disclaimers and policies see 
<a
href="http://www.memetrics.com/emailpolicy.html">http://www.memetrics.com/em
ailpolicy.html</a>

Patch applied. Thanks.

I added #ifdef WIN32 around your new postmaster function.

---------------------------------------------------------------------------

Magnus Hagander wrote:

Hi!

Here's a patch implementing the "thread method" to workaround the bug
with socket calls in signal handlers. See details in mail to
pgsql-hackers-win32 a couple of minutes ago.

//Magnus

<<apc_socket.patch>>

Content-Description: apc_socket.patch

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073