diff --git a/src/backend/port/win32/signal.c b/src/backend/port/win32/signal.c index 4d32a6b54d..e9618b06ec 100644 --- a/src/backend/port/win32/signal.c +++ b/src/backend/port/win32/signal.c @@ -225,64 +225,6 @@ pg_queue_signal(int signum) SetEvent(pgwin32_signal_event); } -/* - * Signal dispatching thread. This runs after we have received a named - * pipe connection from a client (signal sender). Process the request, - * close the pipe, and exit. - */ -static DWORD WINAPI -pg_signal_dispatch_thread(LPVOID param) -{ - HANDLE pipe = (HANDLE) param; - BYTE sigNum; - DWORD bytes; - - if (!ReadFile(pipe, &sigNum, 1, &bytes, NULL)) - { - /* Client died before sending */ - CloseHandle(pipe); - return 0; - } - if (bytes != 1) - { - /* Received bytes over signal pipe (should be 1) */ - CloseHandle(pipe); - return 0; - } - - /* - * Queue the signal before responding to the client. In this way, it's - * guaranteed that once kill() has returned in the signal sender, the next - * CHECK_FOR_INTERRUPTS() in the signal recipient will see the signal. - * (This is a stronger guarantee than POSIX makes; maybe we don't need it? - * But without it, we've seen timing bugs on Windows that do not manifest - * on any known Unix.) - */ - pg_queue_signal(sigNum); - - /* - * Write something back to the client, allowing its CallNamedPipe() call - * to terminate. - */ - WriteFile(pipe, &sigNum, 1, &bytes, NULL); /* Don't care if it works or - * not.. */ - - /* - * We must wait for the client to read the data before we can close the - * pipe, else the data will be lost. (If the WriteFile call failed, - * there'll be nothing in the buffer, so this shouldn't block.) - */ - FlushFileBuffers(pipe); - - /* This is a formality, since we're about to close the pipe anyway. */ - DisconnectNamedPipe(pipe); - - /* And we're done. */ - CloseHandle(pipe); - - return 0; -} - /* Signal handling thread */ static DWORD WINAPI pg_signal_thread(LPVOID param) @@ -290,12 +232,12 @@ pg_signal_thread(LPVOID param) char pipename[128]; HANDLE pipe = pgwin32_initial_signal_pipe; + /* Set up pipe name, in case we have to re-create the pipe. */ snprintf(pipename, sizeof(pipename), "\\\\.\\pipe\\pgsignal_%lu", GetCurrentProcessId()); for (;;) { BOOL fConnected; - HANDLE hThread; /* Create a new pipe instance if we don't have one. */ if (pipe == INVALID_HANDLE_VALUE) @@ -320,59 +262,62 @@ pg_signal_thread(LPVOID param) fConnected = ConnectNamedPipe(pipe, NULL) ? TRUE : (GetLastError() == ERROR_PIPE_CONNECTED); if (fConnected) { - HANDLE newpipe; - /* - * We have a connected pipe. Pass this off to a separate thread - * that will do the actual processing of the pipe. - * - * We must also create a new instance of the pipe *before* we - * start running the new thread. If we don't, there is a race - * condition whereby the dispatch thread might run CloseHandle() - * before we have created a new instance, thereby causing a small - * window of time where we will miss incoming requests. + * We have a connection from a would-be signal sender. Process it. */ - newpipe = CreateNamedPipe(pipename, PIPE_ACCESS_DUPLEX, - PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE | PIPE_WAIT, - PIPE_UNLIMITED_INSTANCES, 16, 16, 1000, NULL); - if (newpipe == INVALID_HANDLE_VALUE) + BYTE sigNum; + DWORD bytes; + + if (ReadFile(pipe, &sigNum, 1, &bytes, NULL) && + bytes == 1) { /* - * This really should never fail. Just retry in case it does, - * even though we have a small race window in that case. There - * is nothing else we can do other than abort the whole - * process which will be even worse. + * Queue the signal before responding to the client. In this + * way, it's guaranteed that once kill() has returned in the + * signal sender, the next CHECK_FOR_INTERRUPTS() in the + * signal recipient will see the signal. (This is a stronger + * guarantee than POSIX makes; maybe we don't need it? But + * without it, we've seen timing bugs on Windows that do not + * manifest on any known Unix.) */ - write_stderr("could not create signal listener pipe: error code %lu; retrying\n", GetLastError()); + pg_queue_signal(sigNum); /* - * Keep going so we at least dispatch this signal. Hopefully, - * the call will succeed when retried in the loop soon after. + * Write something back to the client, allowing its + * CallNamedPipe() call to terminate. + */ + WriteFile(pipe, &sigNum, 1, &bytes, NULL); /* Don't care if it + * works or not */ + + /* + * We must wait for the client to read the data before we can + * disconnect, else the data will be lost. (If the WriteFile + * call failed, there'll be nothing in the buffer, so this + * shouldn't block.) + */ + FlushFileBuffers(pipe); + } + else + { + /* + * If we fail to read a byte from the client, assume it's the + * client's problem and do nothing. Perhaps it'd be better to + * force a pipe close and reopen? */ } - hThread = CreateThread(NULL, 0, - (LPTHREAD_START_ROUTINE) pg_signal_dispatch_thread, - (LPVOID) pipe, 0, NULL); - if (hThread == INVALID_HANDLE_VALUE) - write_stderr("could not create signal dispatch thread: error code %lu\n", - GetLastError()); - else - CloseHandle(hThread); - /* - * Background thread is running with our instance of the pipe. So - * replace our reference with the newly created one and loop back - * up for another run. - */ - pipe = newpipe; + /* Disconnect from client so that we can re-use the pipe. */ + DisconnectNamedPipe(pipe); } else { /* - * Connection failed. Cleanup and try again. + * Connection failed. Cleanup and try again. * - * This should never happen. If it does, we have a small race - * condition until we loop up and re-create the pipe. + * This should never happen. If it does, there's a window where + * we'll miss signals until we manage to re-create the pipe. + * However, just trying to use the same pipe again is probably not + * going to work, so we have little choice. */ CloseHandle(pipe); pipe = INVALID_HANDLE_VALUE;