Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit2ddb914

Browse files
committed
Server-side fix for delayed NOTIFY and SIGTERM processing.
Commit4f85fde introduced some code that was meant to ensure that we'dprocess cancel, die, sinval catchup, and notify interrupts while waitingfor client input. But there was a flaw: it supposed that the processlatch would be set upon arrival at secure_read() if any such interruptwas pending. In reality, we might well have cleared the process latchat some earlier point while those flags remained set -- particularlynotifyInterruptPending, which can't be handled as long as we're withina transaction.To fix the NOTIFY case, also attempt to process signals (exceptProcDiePending) before trying to read.Also, if we see that ProcDiePending is set before we read, forcibly set theprocess latch to ensure that we will handle that signal promptly if no datais available. I also made it set the process latch on the way out, in casethere is similar logic elsewhere. (It remains true that we won't serviceProcDiePending here unless we need to wait for input.)The code for handling ProcDiePending during a write needs those changes,too.Also be a little more careful about when to reset whereToSendOutput,and improve related comments.Back-patch to 9.5 where this code was added. I'm not entirely convincedthat older branches don't have similar issues, but the complaint at handis just about the >= 9.5 code.Jeff Janes and Tom LaneDiscussion:https://postgr.es/m/CAOYf6ec-TmRYjKBXLLaGaB-jrd=mjG1Hzn1a1wufUAR39PQYhw@mail.gmail.com
1 parent12bfb77 commit2ddb914

File tree

2 files changed

+62
-35
lines changed

2 files changed

+62
-35
lines changed

‎src/backend/libpq/be-secure.c

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,9 @@ secure_read(Port *port, void *ptr, size_t len)
145145
ssize_tn;
146146
intwaitfor;
147147

148+
/* Deal with any already-pending interrupt condition. */
149+
ProcessClientReadInterrupt(false);
150+
148151
retry:
149152
#ifdefUSE_SSL
150153
waitfor=0;
@@ -209,9 +212,8 @@ secure_read(Port *port, void *ptr, size_t len)
209212
}
210213

211214
/*
212-
* Process interrupts that happened while (or before) receiving. Note that
213-
* we signal that we're not blocking, which will prevent some types of
214-
* interrupts from being processed.
215+
* Process interrupts that happened during a successful (or non-blocking,
216+
* or hard-failed) read.
215217
*/
216218
ProcessClientReadInterrupt(false);
217219

@@ -248,6 +250,9 @@ secure_write(Port *port, void *ptr, size_t len)
248250
ssize_tn;
249251
intwaitfor;
250252

253+
/* Deal with any already-pending interrupt condition. */
254+
ProcessClientWriteInterrupt(false);
255+
251256
retry:
252257
waitfor=0;
253258
#ifdefUSE_SSL
@@ -287,17 +292,16 @@ secure_write(Port *port, void *ptr, size_t len)
287292

288293
/*
289294
* We'll retry the write. Most likely it will return immediately
290-
* because there's still nodataavailable, and we'll wait for the
291-
* socket to become ready again.
295+
* because there's still nobuffer spaceavailable, and we'll wait
296+
*for thesocket to become ready again.
292297
*/
293298
}
294299
gotoretry;
295300
}
296301

297302
/*
298-
* Process interrupts that happened while (or before) sending. Note that
299-
* we signal that we're not blocking, which will prevent some types of
300-
* interrupts from being processed.
303+
* Process interrupts that happened during a successful (or non-blocking,
304+
* or hard-failed) write.
301305
*/
302306
ProcessClientWriteInterrupt(false);
303307

‎src/backend/tcop/postgres.c

Lines changed: 50 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@ interactive_getc(void)
315315

316316
c=getc(stdin);
317317

318-
ProcessClientReadInterrupt(true);
318+
ProcessClientReadInterrupt(false);
319319

320320
returnc;
321321
}
@@ -520,8 +520,9 @@ ReadCommand(StringInfo inBuf)
520520
/*
521521
* ProcessClientReadInterrupt() - Process interrupts specific to client reads
522522
*
523-
* This is called just after low-level reads. That might be after the read
524-
* finished successfully, or it was interrupted via interrupt.
523+
* This is called just before and after low-level reads.
524+
* 'blocked' is true if no data was available to read and we plan to retry,
525+
* false if about to read or done reading.
525526
*
526527
* Must preserve errno!
527528
*/
@@ -532,23 +533,31 @@ ProcessClientReadInterrupt(bool blocked)
532533

533534
if (DoingCommandRead)
534535
{
535-
/* Check for general interrupts that arrived while reading */
536+
/* Check for general interrupts that arrivedbefore/while reading */
536537
CHECK_FOR_INTERRUPTS();
537538

538-
/* Process sinval catchup interrupts that happened while reading */
539+
/* Process sinval catchup interrupts, if any */
539540
if (catchupInterruptPending)
540541
ProcessCatchupInterrupt();
541542

542-
/* Processsinval catchupinterrupts that happened while reading */
543+
/* Processnotifyinterrupts, if any */
543544
if (notifyInterruptPending)
544545
ProcessNotifyInterrupt();
545546
}
546-
elseif (ProcDiePending&&blocked)
547+
elseif (ProcDiePending)
547548
{
548549
/*
549-
* We're dying. It's safe (and sane) to handle that now.
550+
* We're dying. If there is no data available to read, then it's safe
551+
* (and sane) to handle that now. If we haven't tried to read yet,
552+
* make sure the process latch is set, so that if there is no data
553+
* then we'll come back here and die. If we're done reading, also
554+
* make sure the process latch is set, as we might've undesirably
555+
* cleared it while reading.
550556
*/
551-
CHECK_FOR_INTERRUPTS();
557+
if (blocked)
558+
CHECK_FOR_INTERRUPTS();
559+
else
560+
SetLatch(MyLatch);
552561
}
553562

554563
errno=save_errno;
@@ -557,9 +566,9 @@ ProcessClientReadInterrupt(bool blocked)
557566
/*
558567
* ProcessClientWriteInterrupt() - Process interrupts specific to client writes
559568
*
560-
* This is called just after low-level writes. That might be after the read
561-
*finished successfully, or it was interrupted via interrupt. 'blocked' tells
562-
*us whether the
569+
* This is called justbefore andafter low-level writes.
570+
*'blocked' is true if no data could be written and we plan to retry,
571+
*false if about to write or done writing.
563572
*
564573
* Must preserve errno!
565574
*/
@@ -568,25 +577,39 @@ ProcessClientWriteInterrupt(bool blocked)
568577
{
569578
intsave_errno=errno;
570579

571-
/*
572-
* We only want to process the interrupt here if socket writes are
573-
* blocking to increase the chance to get an error message to the client.
574-
* If we're not blocked there'll soon be a CHECK_FOR_INTERRUPTS(). But if
575-
* we're blocked we'll never get out of that situation if the client has
576-
* died.
577-
*/
578-
if (ProcDiePending&&blocked)
580+
if (ProcDiePending)
579581
{
580582
/*
581-
* We're dying. It's safe (and sane) to handle that now. But we don't
582-
* want to send the client the error message as that a) would possibly
583-
* block again b) would possibly lead to sending an error message to
584-
* the client, while we already started to send something else.
583+
* We're dying. If it's not possible to write, then we should handle
584+
* that immediately, else a stuck client could indefinitely delay our
585+
* response to the signal. If we haven't tried to write yet, make
586+
* sure the process latch is set, so that if the write would block
587+
* then we'll come back here and die. If we're done writing, also
588+
* make sure the process latch is set, as we might've undesirably
589+
* cleared it while writing.
585590
*/
586-
if (whereToSendOutput==DestRemote)
587-
whereToSendOutput=DestNone;
591+
if (blocked)
592+
{
593+
/*
594+
* Don't mess with whereToSendOutput if ProcessInterrupts wouldn't
595+
* do anything.
596+
*/
597+
if (InterruptHoldoffCount==0&&CritSectionCount==0)
598+
{
599+
/*
600+
* We don't want to send the client the error message, as a)
601+
* that would possibly block again, and b) it would likely
602+
* lead to loss of protocol sync because we may have already
603+
* sent a partial protocol message.
604+
*/
605+
if (whereToSendOutput==DestRemote)
606+
whereToSendOutput=DestNone;
588607

589-
CHECK_FOR_INTERRUPTS();
608+
CHECK_FOR_INTERRUPTS();
609+
}
610+
}
611+
else
612+
SetLatch(MyLatch);
590613
}
591614

592615
errno=save_errno;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp