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

Commitfc51a60

Browse files
committed
smgr: Hold interrupts in most smgr functions
We need to hold interrupts across most of the smgr.c/md.c functions, asotherwise interrupt processing, e.g. due to a < ERROR elog/ereport, cantrigger procsignal processing, which in turn can trigger smgrreleaseall(). Asthe relevant code is not reentrant, we quickly end up in a bad situation.The only reason we haven't noticed this before is that there is only onenon-error ereport called in affected routines, in register_dirty_segments(),and that one is extremely rarely reached. If one enables fd.c's FDDEBUG it'seasy to reproduce crashes.It seems better to put the HOLD_INTERRUPTS()/RESUME_INTERRUPTS() in smgr.c,instead of trying to push them down to md.c where possible: For one, everysmgr implementation would be vulnerable, for another, a good bit of smgr.ccode itself is affected too.Eventually we might want a more targeted solution, allowing e.g. a networkedsmgr implementation to be interrupted, but many other, more complicated,problems would need to be fixed for that to be viable (e.g. smgr.c is oftencalled with interrupts already held).One could argue this should be backpatched, but the existing < ERRORelog/ereports that can be reached with unmodified sources are unlikely to bereached. On balance the risk of backpatching seems higher than the gain - atleast for now.Reviewed-by: Noah Misch <noah@leadboat.com>Reviewed-by: Thomas Munro <thomas.munro@gmail.com>Discussion:https://postgr.es/m/3vae7l5ozvqtxmd7rr7zaeq3qkuipz365u3rtim5t5wdkr6f4g@vkgf2fogjirl
1 parentfdb5dd6 commitfc51a60

File tree

1 file changed

+101
-3
lines changed
  • src/backend/storage/smgr

1 file changed

+101
-3
lines changed

‎src/backend/storage/smgr/smgr.c

Lines changed: 101 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,18 @@
4040
* themselves, as there could pointers to them in active use. See
4141
* smgrrelease() and smgrreleaseall().
4242
*
43+
* NB: We need to hold interrupts across most of the functions in this file,
44+
* as otherwise interrupt processing, e.g. due to a < ERROR elog/ereport, can
45+
* trigger procsignal processing, which in turn can trigger
46+
* smgrreleaseall(). Most of the relevant code is not reentrant. It seems
47+
* better to put the HOLD_INTERRUPTS()/RESUME_INTERRUPTS() here, instead of
48+
* trying to push them down to md.c where possible: For one, every smgr
49+
* implementation would be vulnerable, for another, a good bit of smgr.c code
50+
* itself is affected too. Eventually we might want a more targeted solution,
51+
* allowing e.g. a networked smgr implementation to be interrupted, but many
52+
* other, more complicated, problems would need to be fixed for that to be
53+
* viable (e.g. smgr.c is often called with interrupts already held).
54+
*
4355
* Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group
4456
* Portions Copyright (c) 1994, Regents of the University of California
4557
*
@@ -53,6 +65,7 @@
5365

5466
#include"access/xlogutils.h"
5567
#include"lib/ilist.h"
68+
#include"miscadmin.h"
5669
#include"storage/bufmgr.h"
5770
#include"storage/ipc.h"
5871
#include"storage/md.h"
@@ -158,12 +171,16 @@ smgrinit(void)
158171
{
159172
inti;
160173

174+
HOLD_INTERRUPTS();
175+
161176
for (i=0;i<NSmgr;i++)
162177
{
163178
if (smgrsw[i].smgr_init)
164179
smgrsw[i].smgr_init();
165180
}
166181

182+
RESUME_INTERRUPTS();
183+
167184
/* register the shutdown proc */
168185
on_proc_exit(smgrshutdown,0);
169186
}
@@ -176,11 +193,15 @@ smgrshutdown(int code, Datum arg)
176193
{
177194
inti;
178195

196+
HOLD_INTERRUPTS();
197+
179198
for (i=0;i<NSmgr;i++)
180199
{
181200
if (smgrsw[i].smgr_shutdown)
182201
smgrsw[i].smgr_shutdown();
183202
}
203+
204+
RESUME_INTERRUPTS();
184205
}
185206

186207
/*
@@ -206,6 +227,8 @@ smgropen(RelFileLocator rlocator, ProcNumber backend)
206227

207228
Assert(RelFileNumberIsValid(rlocator.relNumber));
208229

230+
HOLD_INTERRUPTS();
231+
209232
if (SMgrRelationHash==NULL)
210233
{
211234
/* First time through: initialize the hash table */
@@ -242,6 +265,8 @@ smgropen(RelFileLocator rlocator, ProcNumber backend)
242265
smgrsw[reln->smgr_which].smgr_open(reln);
243266
}
244267

268+
RESUME_INTERRUPTS();
269+
245270
returnreln;
246271
}
247272

@@ -283,6 +308,8 @@ smgrdestroy(SMgrRelation reln)
283308

284309
Assert(reln->pincount==0);
285310

311+
HOLD_INTERRUPTS();
312+
286313
for (forknum=0;forknum <=MAX_FORKNUM;forknum++)
287314
smgrsw[reln->smgr_which].smgr_close(reln,forknum);
288315

@@ -292,6 +319,8 @@ smgrdestroy(SMgrRelation reln)
292319
&(reln->smgr_rlocator),
293320
HASH_REMOVE,NULL)==NULL)
294321
elog(ERROR,"SMgrRelation hashtable corrupted");
322+
323+
RESUME_INTERRUPTS();
295324
}
296325

297326
/*
@@ -302,12 +331,16 @@ smgrdestroy(SMgrRelation reln)
302331
void
303332
smgrrelease(SMgrRelationreln)
304333
{
334+
HOLD_INTERRUPTS();
335+
305336
for (ForkNumberforknum=0;forknum <=MAX_FORKNUM;forknum++)
306337
{
307338
smgrsw[reln->smgr_which].smgr_close(reln,forknum);
308339
reln->smgr_cached_nblocks[forknum]=InvalidBlockNumber;
309340
}
310341
reln->smgr_targblock=InvalidBlockNumber;
342+
343+
RESUME_INTERRUPTS();
311344
}
312345

313346
/*
@@ -336,6 +369,9 @@ smgrdestroyall(void)
336369
{
337370
dlist_mutable_iteriter;
338371

372+
/* seems unsafe to accept interrupts while in a dlist_foreach_modify() */
373+
HOLD_INTERRUPTS();
374+
339375
/*
340376
* Zap all unpinned SMgrRelations. We rely on smgrdestroy() to remove
341377
* each one from the list.
@@ -347,6 +383,8 @@ smgrdestroyall(void)
347383

348384
smgrdestroy(rel);
349385
}
386+
387+
RESUME_INTERRUPTS();
350388
}
351389

352390
/*
@@ -362,12 +400,17 @@ smgrreleaseall(void)
362400
if (SMgrRelationHash==NULL)
363401
return;
364402

403+
/* seems unsafe to accept interrupts while iterating */
404+
HOLD_INTERRUPTS();
405+
365406
hash_seq_init(&status,SMgrRelationHash);
366407

367408
while ((reln= (SMgrRelation)hash_seq_search(&status))!=NULL)
368409
{
369410
smgrrelease(reln);
370411
}
412+
413+
RESUME_INTERRUPTS();
371414
}
372415

373416
/*
@@ -400,7 +443,13 @@ smgrreleaserellocator(RelFileLocatorBackend rlocator)
400443
bool
401444
smgrexists(SMgrRelationreln,ForkNumberforknum)
402445
{
403-
returnsmgrsw[reln->smgr_which].smgr_exists(reln,forknum);
446+
boolret;
447+
448+
HOLD_INTERRUPTS();
449+
ret=smgrsw[reln->smgr_which].smgr_exists(reln,forknum);
450+
RESUME_INTERRUPTS();
451+
452+
returnret;
404453
}
405454

406455
/*
@@ -413,7 +462,9 @@ smgrexists(SMgrRelation reln, ForkNumber forknum)
413462
void
414463
smgrcreate(SMgrRelationreln,ForkNumberforknum,boolisRedo)
415464
{
465+
HOLD_INTERRUPTS();
416466
smgrsw[reln->smgr_which].smgr_create(reln,forknum,isRedo);
467+
RESUME_INTERRUPTS();
417468
}
418469

419470
/*
@@ -436,6 +487,8 @@ smgrdosyncall(SMgrRelation *rels, int nrels)
436487

437488
FlushRelationsAllBuffers(rels,nrels);
438489

490+
HOLD_INTERRUPTS();
491+
439492
/*
440493
* Sync the physical file(s).
441494
*/
@@ -449,6 +502,8 @@ smgrdosyncall(SMgrRelation *rels, int nrels)
449502
smgrsw[which].smgr_immedsync(rels[i],forknum);
450503
}
451504
}
505+
506+
RESUME_INTERRUPTS();
452507
}
453508

454509
/*
@@ -471,6 +526,13 @@ smgrdounlinkall(SMgrRelation *rels, int nrels, bool isRedo)
471526
if (nrels==0)
472527
return;
473528

529+
/*
530+
* It would be unsafe to process interrupts between DropRelationBuffers()
531+
* and unlinking the underlying files. This probably should be a critical
532+
* section, but we're not there yet.
533+
*/
534+
HOLD_INTERRUPTS();
535+
474536
/*
475537
* Get rid of any remaining buffers for the relations. bufmgr will just
476538
* drop them without bothering to write the contents.
@@ -522,6 +584,8 @@ smgrdounlinkall(SMgrRelation *rels, int nrels, bool isRedo)
522584
}
523585

524586
pfree(rlocators);
587+
588+
RESUME_INTERRUPTS();
525589
}
526590

527591

@@ -538,6 +602,8 @@ void
538602
smgrextend(SMgrRelationreln,ForkNumberforknum,BlockNumberblocknum,
539603
constvoid*buffer,boolskipFsync)
540604
{
605+
HOLD_INTERRUPTS();
606+
541607
smgrsw[reln->smgr_which].smgr_extend(reln,forknum,blocknum,
542608
buffer,skipFsync);
543609

@@ -550,6 +616,8 @@ smgrextend(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,
550616
reln->smgr_cached_nblocks[forknum]=blocknum+1;
551617
else
552618
reln->smgr_cached_nblocks[forknum]=InvalidBlockNumber;
619+
620+
RESUME_INTERRUPTS();
553621
}
554622

555623
/*
@@ -563,6 +631,8 @@ void
563631
smgrzeroextend(SMgrRelationreln,ForkNumberforknum,BlockNumberblocknum,
564632
intnblocks,boolskipFsync)
565633
{
634+
HOLD_INTERRUPTS();
635+
566636
smgrsw[reln->smgr_which].smgr_zeroextend(reln,forknum,blocknum,
567637
nblocks,skipFsync);
568638

@@ -575,6 +645,8 @@ smgrzeroextend(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,
575645
reln->smgr_cached_nblocks[forknum]=blocknum+nblocks;
576646
else
577647
reln->smgr_cached_nblocks[forknum]=InvalidBlockNumber;
648+
649+
RESUME_INTERRUPTS();
578650
}
579651

580652
/*
@@ -588,7 +660,13 @@ bool
588660
smgrprefetch(SMgrRelationreln,ForkNumberforknum,BlockNumberblocknum,
589661
intnblocks)
590662
{
591-
returnsmgrsw[reln->smgr_which].smgr_prefetch(reln,forknum,blocknum,nblocks);
663+
boolret;
664+
665+
HOLD_INTERRUPTS();
666+
ret=smgrsw[reln->smgr_which].smgr_prefetch(reln,forknum,blocknum,nblocks);
667+
RESUME_INTERRUPTS();
668+
669+
returnret;
592670
}
593671

594672
/*
@@ -601,7 +679,13 @@ uint32
601679
smgrmaxcombine(SMgrRelationreln,ForkNumberforknum,
602680
BlockNumberblocknum)
603681
{
604-
returnsmgrsw[reln->smgr_which].smgr_maxcombine(reln,forknum,blocknum);
682+
uint32ret;
683+
684+
HOLD_INTERRUPTS();
685+
ret=smgrsw[reln->smgr_which].smgr_maxcombine(reln,forknum,blocknum);
686+
RESUME_INTERRUPTS();
687+
688+
returnret;
605689
}
606690

607691
/*
@@ -619,8 +703,10 @@ void
619703
smgrreadv(SMgrRelationreln,ForkNumberforknum,BlockNumberblocknum,
620704
void**buffers,BlockNumbernblocks)
621705
{
706+
HOLD_INTERRUPTS();
622707
smgrsw[reln->smgr_which].smgr_readv(reln,forknum,blocknum,buffers,
623708
nblocks);
709+
RESUME_INTERRUPTS();
624710
}
625711

626712
/*
@@ -653,8 +739,10 @@ void
653739
smgrwritev(SMgrRelationreln,ForkNumberforknum,BlockNumberblocknum,
654740
constvoid**buffers,BlockNumbernblocks,boolskipFsync)
655741
{
742+
HOLD_INTERRUPTS();
656743
smgrsw[reln->smgr_which].smgr_writev(reln,forknum,blocknum,
657744
buffers,nblocks,skipFsync);
745+
RESUME_INTERRUPTS();
658746
}
659747

660748
/*
@@ -665,8 +753,10 @@ void
665753
smgrwriteback(SMgrRelationreln,ForkNumberforknum,BlockNumberblocknum,
666754
BlockNumbernblocks)
667755
{
756+
HOLD_INTERRUPTS();
668757
smgrsw[reln->smgr_which].smgr_writeback(reln,forknum,blocknum,
669758
nblocks);
759+
RESUME_INTERRUPTS();
670760
}
671761

672762
/*
@@ -683,10 +773,14 @@ smgrnblocks(SMgrRelation reln, ForkNumber forknum)
683773
if (result!=InvalidBlockNumber)
684774
returnresult;
685775

776+
HOLD_INTERRUPTS();
777+
686778
result=smgrsw[reln->smgr_which].smgr_nblocks(reln,forknum);
687779

688780
reln->smgr_cached_nblocks[forknum]=result;
689781

782+
RESUME_INTERRUPTS();
783+
690784
returnresult;
691785
}
692786

@@ -784,7 +878,9 @@ smgrtruncate(SMgrRelation reln, ForkNumber *forknum, int nforks,
784878
void
785879
smgrregistersync(SMgrRelationreln,ForkNumberforknum)
786880
{
881+
HOLD_INTERRUPTS();
787882
smgrsw[reln->smgr_which].smgr_registersync(reln,forknum);
883+
RESUME_INTERRUPTS();
788884
}
789885

790886
/*
@@ -816,7 +912,9 @@ smgrregistersync(SMgrRelation reln, ForkNumber forknum)
816912
void
817913
smgrimmedsync(SMgrRelationreln,ForkNumberforknum)
818914
{
915+
HOLD_INTERRUPTS();
819916
smgrsw[reln->smgr_which].smgr_immedsync(reln,forknum);
917+
RESUME_INTERRUPTS();
820918
}
821919

822920
/*

0 commit comments

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