Обсуждение: Patch for select and APC on win32
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>>
Вложения
> 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