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

Commitd466335

Browse files
committed
Don't be so trusting that shm_toc_lookup() will always succeed.
Given the possibility of race conditions and so on, it seems entirelyunsafe to just assume that shm_toc_lookup() always finds the key it'slooking for --- but that was exactly what all but one call site weredoing. To fix, add a "bool noError" argument, similarly to what wehave in many other functions, and throw an error on an unexpectedlookup failure. Remove now-redundant Asserts that a rather randomsubset of call sites had.I doubt this will throw any light on buildfarm member lorikeet'srecent failures, because if an unnoticed lookup failure were involved,you'd kind of expect a null-pointer-dereference crash rather than theobserved symptom. But you never know ... and this is better codingpractice even if it never catches anything.Discussion:https://postgr.es/m/9697.1496675981@sss.pgh.pa.us
1 parentaf51fea commitd466335

File tree

11 files changed

+39
-38
lines changed

11 files changed

+39
-38
lines changed

‎src/backend/access/transam/parallel.c‎

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -392,12 +392,12 @@ ReinitializeParallelDSM(ParallelContext *pcxt)
392392
}
393393

394394
/* Reset a few bits of fixed parallel state to a clean state. */
395-
fps=shm_toc_lookup(pcxt->toc,PARALLEL_KEY_FIXED);
395+
fps=shm_toc_lookup(pcxt->toc,PARALLEL_KEY_FIXED, false);
396396
fps->last_xlog_end=0;
397397

398398
/* Recreate error queues. */
399399
error_queue_space=
400-
shm_toc_lookup(pcxt->toc,PARALLEL_KEY_ERROR_QUEUE);
400+
shm_toc_lookup(pcxt->toc,PARALLEL_KEY_ERROR_QUEUE, false);
401401
for (i=0;i<pcxt->nworkers;++i)
402402
{
403403
char*start;
@@ -536,7 +536,7 @@ WaitForParallelWorkersToFinish(ParallelContext *pcxt)
536536
{
537537
FixedParallelState*fps;
538538

539-
fps=shm_toc_lookup(pcxt->toc,PARALLEL_KEY_FIXED);
539+
fps=shm_toc_lookup(pcxt->toc,PARALLEL_KEY_FIXED, false);
540540
if (fps->last_xlog_end>XactLastRecEnd)
541541
XactLastRecEnd=fps->last_xlog_end;
542542
}
@@ -973,8 +973,7 @@ ParallelWorkerMain(Datum main_arg)
973973
errmsg("invalid magic number in dynamic shared memory segment")));
974974

975975
/* Look up fixed parallel state. */
976-
fps=shm_toc_lookup(toc,PARALLEL_KEY_FIXED);
977-
Assert(fps!=NULL);
976+
fps=shm_toc_lookup(toc,PARALLEL_KEY_FIXED, false);
978977
MyFixedParallelState=fps;
979978

980979
/*
@@ -983,7 +982,7 @@ ParallelWorkerMain(Datum main_arg)
983982
* errors that happen here will not be reported back to the process that
984983
* requested that this worker be launched.
985984
*/
986-
error_queue_space=shm_toc_lookup(toc,PARALLEL_KEY_ERROR_QUEUE);
985+
error_queue_space=shm_toc_lookup(toc,PARALLEL_KEY_ERROR_QUEUE, false);
987986
mq= (shm_mq*) (error_queue_space+
988987
ParallelWorkerNumber*PARALLEL_ERROR_QUEUE_SIZE);
989988
shm_mq_set_sender(mq,MyProc);
@@ -1027,17 +1026,15 @@ ParallelWorkerMain(Datum main_arg)
10271026
* this before restoring GUCs, because the libraries might define custom
10281027
* variables.
10291028
*/
1030-
libraryspace=shm_toc_lookup(toc,PARALLEL_KEY_LIBRARY);
1031-
Assert(libraryspace!=NULL);
1029+
libraryspace=shm_toc_lookup(toc,PARALLEL_KEY_LIBRARY, false);
10321030
RestoreLibraryState(libraryspace);
10331031

10341032
/*
10351033
* Identify the entry point to be called. In theory this could result in
10361034
* loading an additional library, though most likely the entry point is in
10371035
* the core backend or in a library we just loaded.
10381036
*/
1039-
entrypointstate=shm_toc_lookup(toc,PARALLEL_KEY_ENTRYPOINT);
1040-
Assert(entrypointstate!=NULL);
1037+
entrypointstate=shm_toc_lookup(toc,PARALLEL_KEY_ENTRYPOINT, false);
10411038
library_name=entrypointstate;
10421039
function_name=entrypointstate+strlen(library_name)+1;
10431040

@@ -1054,30 +1051,26 @@ ParallelWorkerMain(Datum main_arg)
10541051
SetClientEncoding(GetDatabaseEncoding());
10551052

10561053
/* Restore GUC values from launching backend. */
1057-
gucspace=shm_toc_lookup(toc,PARALLEL_KEY_GUC);
1058-
Assert(gucspace!=NULL);
1054+
gucspace=shm_toc_lookup(toc,PARALLEL_KEY_GUC, false);
10591055
StartTransactionCommand();
10601056
RestoreGUCState(gucspace);
10611057
CommitTransactionCommand();
10621058

10631059
/* Crank up a transaction state appropriate to a parallel worker. */
1064-
tstatespace=shm_toc_lookup(toc,PARALLEL_KEY_TRANSACTION_STATE);
1060+
tstatespace=shm_toc_lookup(toc,PARALLEL_KEY_TRANSACTION_STATE, false);
10651061
StartParallelWorkerTransaction(tstatespace);
10661062

10671063
/* Restore combo CID state. */
1068-
combocidspace=shm_toc_lookup(toc,PARALLEL_KEY_COMBO_CID);
1069-
Assert(combocidspace!=NULL);
1064+
combocidspace=shm_toc_lookup(toc,PARALLEL_KEY_COMBO_CID, false);
10701065
RestoreComboCIDState(combocidspace);
10711066

10721067
/* Restore transaction snapshot. */
1073-
tsnapspace=shm_toc_lookup(toc,PARALLEL_KEY_TRANSACTION_SNAPSHOT);
1074-
Assert(tsnapspace!=NULL);
1068+
tsnapspace=shm_toc_lookup(toc,PARALLEL_KEY_TRANSACTION_SNAPSHOT, false);
10751069
RestoreTransactionSnapshot(RestoreSnapshot(tsnapspace),
10761070
fps->parallel_master_pgproc);
10771071

10781072
/* Restore active snapshot. */
1079-
asnapspace=shm_toc_lookup(toc,PARALLEL_KEY_ACTIVE_SNAPSHOT);
1080-
Assert(asnapspace!=NULL);
1073+
asnapspace=shm_toc_lookup(toc,PARALLEL_KEY_ACTIVE_SNAPSHOT, false);
10811074
PushActiveSnapshot(RestoreSnapshot(asnapspace));
10821075

10831076
/*

‎src/backend/executor/execParallel.c‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ ExecParallelSetupTupleQueues(ParallelContext *pcxt, bool reinitialize)
341341
mul_size(PARALLEL_TUPLE_QUEUE_SIZE,
342342
pcxt->nworkers));
343343
else
344-
tqueuespace=shm_toc_lookup(pcxt->toc,PARALLEL_KEY_TUPLE_QUEUE);
344+
tqueuespace=shm_toc_lookup(pcxt->toc,PARALLEL_KEY_TUPLE_QUEUE, false);
345345

346346
/* Create the queues, and become the receiver for each. */
347347
for (i=0;i<pcxt->nworkers;++i)
@@ -684,7 +684,7 @@ ExecParallelGetReceiver(dsm_segment *seg, shm_toc *toc)
684684
char*mqspace;
685685
shm_mq*mq;
686686

687-
mqspace=shm_toc_lookup(toc,PARALLEL_KEY_TUPLE_QUEUE);
687+
mqspace=shm_toc_lookup(toc,PARALLEL_KEY_TUPLE_QUEUE, false);
688688
mqspace+=ParallelWorkerNumber*PARALLEL_TUPLE_QUEUE_SIZE;
689689
mq= (shm_mq*)mqspace;
690690
shm_mq_set_sender(mq,MyProc);
@@ -705,14 +705,14 @@ ExecParallelGetQueryDesc(shm_toc *toc, DestReceiver *receiver,
705705
char*queryString;
706706

707707
/* Get the query string from shared memory */
708-
queryString=shm_toc_lookup(toc,PARALLEL_KEY_QUERY_TEXT);
708+
queryString=shm_toc_lookup(toc,PARALLEL_KEY_QUERY_TEXT, false);
709709

710710
/* Reconstruct leader-supplied PlannedStmt. */
711-
pstmtspace=shm_toc_lookup(toc,PARALLEL_KEY_PLANNEDSTMT);
711+
pstmtspace=shm_toc_lookup(toc,PARALLEL_KEY_PLANNEDSTMT, false);
712712
pstmt= (PlannedStmt*)stringToNode(pstmtspace);
713713

714714
/* Reconstruct ParamListInfo. */
715-
paramspace=shm_toc_lookup(toc,PARALLEL_KEY_PARAMS);
715+
paramspace=shm_toc_lookup(toc,PARALLEL_KEY_PARAMS, false);
716716
paramLI=RestoreParamList(&paramspace);
717717

718718
/*
@@ -843,7 +843,7 @@ ParallelQueryMain(dsm_segment *seg, shm_toc *toc)
843843

844844
/* Set up DestReceiver, SharedExecutorInstrumentation, and QueryDesc. */
845845
receiver=ExecParallelGetReceiver(seg,toc);
846-
instrumentation=shm_toc_lookup(toc,PARALLEL_KEY_INSTRUMENTATION);
846+
instrumentation=shm_toc_lookup(toc,PARALLEL_KEY_INSTRUMENTATION, true);
847847
if (instrumentation!=NULL)
848848
instrument_options=instrumentation->instrument_options;
849849
queryDesc=ExecParallelGetQueryDesc(toc,receiver,instrument_options);
@@ -858,7 +858,7 @@ ParallelQueryMain(dsm_segment *seg, shm_toc *toc)
858858
InstrStartParallelQuery();
859859

860860
/* Attach to the dynamic shared memory area. */
861-
area_space=shm_toc_lookup(toc,PARALLEL_KEY_DSA);
861+
area_space=shm_toc_lookup(toc,PARALLEL_KEY_DSA, false);
862862
area=dsa_attach_in_place(area_space,seg);
863863

864864
/* Start up the executor */
@@ -875,7 +875,7 @@ ParallelQueryMain(dsm_segment *seg, shm_toc *toc)
875875
ExecutorFinish(queryDesc);
876876

877877
/* Report buffer usage during parallel execution. */
878-
buffer_usage=shm_toc_lookup(toc,PARALLEL_KEY_BUFFER_USAGE);
878+
buffer_usage=shm_toc_lookup(toc,PARALLEL_KEY_BUFFER_USAGE, false);
879879
InstrEndParallelQuery(&buffer_usage[ParallelWorkerNumber]);
880880

881881
/* Report instrumentation data if any instrumentation options are set. */

‎src/backend/executor/nodeBitmapHeapscan.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1005,7 +1005,7 @@ ExecBitmapHeapInitializeWorker(BitmapHeapScanState *node, shm_toc *toc)
10051005
ParallelBitmapHeapState*pstate;
10061006
Snapshotsnapshot;
10071007

1008-
pstate=shm_toc_lookup(toc,node->ss.ps.plan->plan_node_id);
1008+
pstate=shm_toc_lookup(toc,node->ss.ps.plan->plan_node_id, false);
10091009
node->pstate=pstate;
10101010

10111011
snapshot=RestoreSnapshot(pstate->phs_snapshot_data);

‎src/backend/executor/nodeCustom.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ ExecCustomScanInitializeWorker(CustomScanState *node, shm_toc *toc)
194194
intplan_node_id=node->ss.ps.plan->plan_node_id;
195195
void*coordinate;
196196

197-
coordinate=shm_toc_lookup(toc,plan_node_id);
197+
coordinate=shm_toc_lookup(toc,plan_node_id, false);
198198
methods->InitializeWorkerCustomScan(node,toc,coordinate);
199199
}
200200
}

‎src/backend/executor/nodeForeignscan.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,7 @@ ExecForeignScanInitializeWorker(ForeignScanState *node, shm_toc *toc)
344344
intplan_node_id=node->ss.ps.plan->plan_node_id;
345345
void*coordinate;
346346

347-
coordinate=shm_toc_lookup(toc,plan_node_id);
347+
coordinate=shm_toc_lookup(toc,plan_node_id, false);
348348
fdwroutine->InitializeWorkerForeignScan(node,toc,coordinate);
349349
}
350350
}

‎src/backend/executor/nodeIndexonlyscan.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -676,7 +676,7 @@ ExecIndexOnlyScanInitializeWorker(IndexOnlyScanState *node, shm_toc *toc)
676676
{
677677
ParallelIndexScanDescpiscan;
678678

679-
piscan=shm_toc_lookup(toc,node->ss.ps.plan->plan_node_id);
679+
piscan=shm_toc_lookup(toc,node->ss.ps.plan->plan_node_id, false);
680680
node->ioss_ScanDesc=
681681
index_beginscan_parallel(node->ss.ss_currentRelation,
682682
node->ioss_RelationDesc,

‎src/backend/executor/nodeIndexscan.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1714,7 +1714,7 @@ ExecIndexScanInitializeWorker(IndexScanState *node, shm_toc *toc)
17141714
{
17151715
ParallelIndexScanDescpiscan;
17161716

1717-
piscan=shm_toc_lookup(toc,node->ss.ps.plan->plan_node_id);
1717+
piscan=shm_toc_lookup(toc,node->ss.ps.plan->plan_node_id, false);
17181718
node->iss_ScanDesc=
17191719
index_beginscan_parallel(node->ss.ss_currentRelation,
17201720
node->iss_RelationDesc,

‎src/backend/executor/nodeSeqscan.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ ExecSeqScanInitializeWorker(SeqScanState *node, shm_toc *toc)
332332
{
333333
ParallelHeapScanDescpscan;
334334

335-
pscan=shm_toc_lookup(toc,node->ss.ps.plan->plan_node_id);
335+
pscan=shm_toc_lookup(toc,node->ss.ps.plan->plan_node_id, false);
336336
node->ss.ss_currentScanDesc=
337337
heap_beginscan_parallel(node->ss.ss_currentRelation,pscan);
338338
}

‎src/backend/storage/ipc/shm_toc.c‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,14 +208,17 @@ shm_toc_insert(shm_toc *toc, uint64 key, void *address)
208208
/*
209209
* Look up a TOC entry.
210210
*
211+
* If the key is not found, returns NULL if noError is true, otherwise
212+
* throws elog(ERROR).
213+
*
211214
* Unlike the other functions in this file, this operation acquires no lock;
212215
* it uses only barriers. It probably wouldn't hurt concurrency very much even
213216
* if it did get a lock, but since it's reasonably likely that a group of
214217
* worker processes could each read a series of entries from the same TOC
215218
* right around the same time, there seems to be some value in avoiding it.
216219
*/
217220
void*
218-
shm_toc_lookup(shm_toc*toc,uint64key)
221+
shm_toc_lookup(shm_toc*toc,uint64key,boolnoError)
219222
{
220223
uint64nentry;
221224
uint64i;
@@ -226,10 +229,15 @@ shm_toc_lookup(shm_toc *toc, uint64 key)
226229

227230
/* Now search for a matching entry. */
228231
for (i=0;i<nentry;++i)
232+
{
229233
if (toc->toc_entry[i].key==key)
230234
return ((char*)toc)+toc->toc_entry[i].offset;
235+
}
231236

232237
/* No matching entry was found. */
238+
if (!noError)
239+
elog(ERROR,"could not find key "UINT64_FORMAT" in shm TOC at %p",
240+
key,toc);
233241
returnNULL;
234242
}
235243

‎src/include/storage/shm_toc.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ extern shm_toc *shm_toc_attach(uint64 magic, void *address);
3232
externvoid*shm_toc_allocate(shm_toc*toc,Sizenbytes);
3333
externSizeshm_toc_freespace(shm_toc*toc);
3434
externvoidshm_toc_insert(shm_toc*toc,uint64key,void*address);
35-
externvoid*shm_toc_lookup(shm_toc*toc,uint64key);
35+
externvoid*shm_toc_lookup(shm_toc*toc,uint64key,boolnoError);
3636

3737
/*
3838
* Tools for estimating how large a chunk of shared memory will be needed

0 commit comments

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