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

Commit7aaeb7b

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 parent9892c18 commit7aaeb7b

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
@@ -144,6 +144,9 @@ secure_read(Port *port, void *ptr, size_t len)
144144
ssize_tn;
145145
intwaitfor;
146146

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

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

@@ -247,6 +249,9 @@ secure_write(Port *port, void *ptr, size_t len)
247249
ssize_tn;
248250
intwaitfor;
249251

252+
/* Deal with any already-pending interrupt condition. */
253+
ProcessClientWriteInterrupt(false);
254+
250255
retry:
251256
waitfor=0;
252257
#ifdefUSE_SSL
@@ -286,17 +291,16 @@ secure_write(Port *port, void *ptr, size_t len)
286291

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

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

‎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