Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit ada8fa0

Browse files
committed
Fix Windows implementation of PGSemaphoreLock.
The original coding failed to reset ImmediateInterruptOK before returning, which would potentially allow a subsequent query-cancel interrupt to be accepted at an unsafe point. This is a really nasty bug since it's so hard to predict the consequences, but they could be unpleasant. Also, ensure that signal handlers are serviced before this function returns, even if the semaphore is already set. This should make the behavior more like Unix. Back-patch to all supported versions.
1 parent 8ebc908 commit ada8fa0

File tree

1 file changed

+12
-8
lines changed

1 file changed

+12
-8
lines changed

src/backend/port/win32_sema.c

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -121,8 +121,13 @@ PGSemaphoreLock(PGSemaphore sema, bool interruptOK)
121121
DWORD ret;
122122
HANDLE wh[2];
123123

124-
wh[0] = *sema;
125-
wh[1] = pgwin32_signal_event;
124+
/*
125+
* Note: pgwin32_signal_event should be first to ensure that it will be
126+
* reported when multiple events are set. We want to guarantee that
127+
* pending signals are serviced.
128+
*/
129+
wh[0] = pgwin32_signal_event;
130+
wh[1] = *sema;
126131

127132
/*
128133
* As in other implementations of PGSemaphoreLock, we need to check for
@@ -135,20 +140,19 @@ PGSemaphoreLock(PGSemaphore sema, bool interruptOK)
135140
ImmediateInterruptOK = interruptOK;
136141
CHECK_FOR_INTERRUPTS();
137142

138-
errno = 0;
139143
ret = WaitForMultipleObjectsEx(2, wh, FALSE, INFINITE, TRUE);
140144

141145
if (ret == WAIT_OBJECT_0)
142-
{
143-
/* We got it! */
144-
return;
145-
}
146-
else if (ret == WAIT_OBJECT_0 + 1)
147146
{
148147
/* Signal event is set - we have a signal to deliver */
149148
pgwin32_dispatch_queued_signals();
150149
errno = EINTR;
151150
}
151+
else if (ret == WAIT_OBJECT_0 + 1)
152+
{
153+
/* We got it! */
154+
errno = 0;
155+
}
152156
else
153157
/* Otherwise we are in trouble */
154158
errno = EIDRM;

0 commit comments

Comments
 (0)