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

Commit04ef202

Browse files
committed
Fix Portal snapshot tracking to handle subtransactions properly.
Commit84f5c29 forgot to consider the possibility thatEnsurePortalSnapshotExists could run inside a subtransaction withlifespan shorter than the Portal's. In that case, the new activesnapshot would be popped at the end of the subtransaction, leavinga dangling pointer in the Portal, with mayhem ensuing.To fix, make sure the ActiveSnapshot stack entry is marked withthe same subtransaction nesting level as the associated Portal.It's certainly safe to do so since we won't be here at all unlessthe stack is empty; hence we can't create an out-of-order stack.Let's also apply this logic in the case where PortalRunUtilitysets portalSnapshot, just to be sure that path can't cause similarproblems. It's slightly less clear that that path can't createan out-of-order stack, so add an assertion guarding it.Report and patch by Bertrand Drouvot (with kibitzing by me).Back-patch to v11, like the previous commit.Discussion:https://postgr.es/m/ff82b8c5-77f4-3fe7-6028-fcf3303e82dd@amazon.com
1 parent649e561 commit04ef202

File tree

8 files changed

+95
-8
lines changed

8 files changed

+95
-8
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4842,6 +4842,7 @@ CommitSubTransaction(void)
48424842
AfterTriggerEndSubXact(true);
48434843
AtSubCommit_Portals(s->subTransactionId,
48444844
s->parent->subTransactionId,
4845+
s->parent->nestingLevel,
48454846
s->parent->curTransactionOwner);
48464847
AtEOSubXact_LargeObject(true,s->subTransactionId,
48474848
s->parent->subTransactionId);

‎src/backend/tcop/pquery.c

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -480,7 +480,9 @@ PortalStart(Portal portal, ParamListInfo params,
480480
* We could remember the snapshot in portal->portalSnapshot,
481481
* but presently there seems no need to, as this code path
482482
* cannot be used for non-atomic execution. Hence there can't
483-
* be any commit/abort that might destroy the snapshot.
483+
* be any commit/abort that might destroy the snapshot. Since
484+
* we don't do that, there's also no need to force a
485+
* non-default nesting level for the snapshot.
484486
*/
485487

486488
/*
@@ -1134,9 +1136,15 @@ PortalRunUtility(Portal portal, PlannedStmt *pstmt,
11341136
snapshot=RegisterSnapshot(snapshot);
11351137
portal->holdSnapshot=snapshot;
11361138
}
1137-
/* In any case, make the snapshot active and remember it in portal */
1138-
PushActiveSnapshot(snapshot);
1139-
/* PushActiveSnapshot might have copied the snapshot */
1139+
1140+
/*
1141+
* In any case, make the snapshot active and remember it in portal.
1142+
* Because the portal now references the snapshot, we must tell
1143+
* snapmgr.c that the snapshot belongs to the portal's transaction
1144+
* level, else we risk portalSnapshot becoming a dangling pointer.
1145+
*/
1146+
PushActiveSnapshotWithLevel(snapshot,portal->createLevel);
1147+
/* PushActiveSnapshotWithLevel might have copied the snapshot */
11401148
portal->portalSnapshot=GetActiveSnapshot();
11411149
}
11421150
else
@@ -1786,8 +1794,13 @@ EnsurePortalSnapshotExists(void)
17861794
elog(ERROR,"cannot execute SQL without an outer snapshot or portal");
17871795
Assert(portal->portalSnapshot==NULL);
17881796

1789-
/* Create a new snapshot and make it active */
1790-
PushActiveSnapshot(GetTransactionSnapshot());
1791-
/* PushActiveSnapshot might have copied the snapshot */
1797+
/*
1798+
* Create a new snapshot, make it active, and remember it in portal.
1799+
* Because the portal now references the snapshot, we must tell snapmgr.c
1800+
* that the snapshot belongs to the portal's transaction level, else we
1801+
* risk portalSnapshot becoming a dangling pointer.
1802+
*/
1803+
PushActiveSnapshotWithLevel(GetTransactionSnapshot(),portal->createLevel);
1804+
/* PushActiveSnapshotWithLevel might have copied the snapshot */
17921805
portal->portalSnapshot=GetActiveSnapshot();
17931806
}

‎src/backend/utils/mmgr/portalmem.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,7 @@ CreatePortal(const char *name, bool allowDup, bool dupSilent)
210210
portal->cleanup=PortalCleanup;
211211
portal->createSubid=GetCurrentSubTransactionId();
212212
portal->activeSubid=portal->createSubid;
213+
portal->createLevel=GetCurrentTransactionNestLevel();
213214
portal->strategy=PORTAL_MULTI_QUERY;
214215
portal->cursorOptions=CURSOR_OPT_NO_SCROLL;
215216
portal->atStart= true;
@@ -657,6 +658,7 @@ HoldPortal(Portal portal)
657658
*/
658659
portal->createSubid=InvalidSubTransactionId;
659660
portal->activeSubid=InvalidSubTransactionId;
661+
portal->createLevel=0;
660662
}
661663

662664
/*
@@ -940,6 +942,7 @@ PortalErrorCleanup(void)
940942
void
941943
AtSubCommit_Portals(SubTransactionIdmySubid,
942944
SubTransactionIdparentSubid,
945+
intparentLevel,
943946
ResourceOwnerparentXactOwner)
944947
{
945948
HASH_SEQ_STATUSstatus;
@@ -954,6 +957,7 @@ AtSubCommit_Portals(SubTransactionId mySubid,
954957
if (portal->createSubid==mySubid)
955958
{
956959
portal->createSubid=parentSubid;
960+
portal->createLevel=parentLevel;
957961
if (portal->resowner)
958962
ResourceOwnerNewParent(portal->resowner,parentXactOwner);
959963
}

‎src/backend/utils/time/snapmgr.c

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -733,10 +733,25 @@ FreeSnapshot(Snapshot snapshot)
733733
*/
734734
void
735735
PushActiveSnapshot(Snapshotsnap)
736+
{
737+
PushActiveSnapshotWithLevel(snap,GetCurrentTransactionNestLevel());
738+
}
739+
740+
/*
741+
* PushActiveSnapshotWithLevel
742+
*Set the given snapshot as the current active snapshot
743+
*
744+
* Same as PushActiveSnapshot except that caller can specify the
745+
* transaction nesting level that "owns" the snapshot. This level
746+
* must not be deeper than the current top of the snapshot stack.
747+
*/
748+
void
749+
PushActiveSnapshotWithLevel(Snapshotsnap,intsnap_level)
736750
{
737751
ActiveSnapshotElt*newactive;
738752

739753
Assert(snap!=InvalidSnapshot);
754+
Assert(ActiveSnapshot==NULL||snap_level >=ActiveSnapshot->as_level);
740755

741756
newactive=MemoryContextAlloc(TopTransactionContext,sizeof(ActiveSnapshotElt));
742757

@@ -750,7 +765,7 @@ PushActiveSnapshot(Snapshot snap)
750765
newactive->as_snap=snap;
751766

752767
newactive->as_next=ActiveSnapshot;
753-
newactive->as_level=GetCurrentTransactionNestLevel();
768+
newactive->as_level=snap_level;
754769

755770
newactive->as_snap->active_count++;
756771

‎src/include/utils/portal.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,13 +195,16 @@ typedef struct PortalData
195195
TimestampTzcreation_time;/* time at which this portal was defined */
196196
boolvisible;/* include this portal in pg_cursors? */
197197

198+
/* Stuff added at the end to avoid ABI break in stable branches: */
199+
198200
/*
199201
* Outermost ActiveSnapshot for execution of the portal's queries. For
200202
* all but a few utility commands, we require such a snapshot to exist.
201203
* This ensures that TOAST references in query results can be detoasted,
202204
* and helps to reduce thrashing of the process's exposed xmin.
203205
*/
204206
SnapshotportalSnapshot;/* active snapshot, or NULL if none */
207+
intcreateLevel;/* creating subxact's nesting level */
205208
}PortalData;
206209

207210
/*
@@ -219,6 +222,7 @@ extern void AtCleanup_Portals(void);
219222
externvoidPortalErrorCleanup(void);
220223
externvoidAtSubCommit_Portals(SubTransactionIdmySubid,
221224
SubTransactionIdparentSubid,
225+
intparentLevel,
222226
ResourceOwnerparentXactOwner);
223227
externvoidAtSubAbort_Portals(SubTransactionIdmySubid,
224228
SubTransactionIdparentSubid,

‎src/include/utils/snapmgr.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ extern void InvalidateCatalogSnapshot(void);
110110
externvoidInvalidateCatalogSnapshotConditionally(void);
111111

112112
externvoidPushActiveSnapshot(Snapshotsnapshot);
113+
externvoidPushActiveSnapshotWithLevel(Snapshotsnapshot,intsnap_level);
113114
externvoidPushCopiedSnapshot(Snapshotsnapshot);
114115
externvoidUpdateActiveSnapshotCommandId(void);
115116
externvoidPopActiveSnapshot(void);

‎src/pl/plpgsql/src/expected/plpgsql_transaction.out

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,34 @@ SELECT * FROM test1;
430430
---+---
431431
(0 rows)
432432

433+
-- test commit/rollback inside exception handler, too
434+
TRUNCATE test1;
435+
DO LANGUAGE plpgsql $$
436+
BEGIN
437+
FOR i IN 1..10 LOOP
438+
BEGIN
439+
INSERT INTO test1 VALUES (i, 'good');
440+
INSERT INTO test1 VALUES (i/0, 'bad');
441+
EXCEPTION
442+
WHEN division_by_zero THEN
443+
INSERT INTO test1 VALUES (i, 'exception');
444+
IF (i % 3) > 0 THEN COMMIT; ELSE ROLLBACK; END IF;
445+
END;
446+
END LOOP;
447+
END;
448+
$$;
449+
SELECT * FROM test1;
450+
a | b
451+
----+-----------
452+
1 | exception
453+
2 | exception
454+
4 | exception
455+
5 | exception
456+
7 | exception
457+
8 | exception
458+
10 | exception
459+
(7 rows)
460+
433461
-- detoast result of simple expression after commit
434462
CREATE TEMP TABLE test4(f1 text);
435463
ALTER TABLE test4 ALTER COLUMN f1 SET STORAGE EXTERNAL; -- disable compression

‎src/pl/plpgsql/src/sql/plpgsql_transaction.sql

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,27 @@ $$;
354354
SELECT*FROM test1;
355355

356356

357+
-- test commit/rollback inside exception handler, too
358+
TRUNCATE test1;
359+
360+
DO LANGUAGE plpgsql $$
361+
BEGIN
362+
FOR iIN1..10 LOOP
363+
BEGIN
364+
INSERT INTO test1VALUES (i,'good');
365+
INSERT INTO test1VALUES (i/0,'bad');
366+
EXCEPTION
367+
WHEN division_by_zero THEN
368+
INSERT INTO test1VALUES (i,'exception');
369+
IF (i %3)>0 THENCOMMIT; ELSEROLLBACK; END IF;
370+
END;
371+
END LOOP;
372+
END;
373+
$$;
374+
375+
SELECT*FROM test1;
376+
377+
357378
-- detoast result of simple expression after commit
358379
CREATE TEMP TABLE test4(f1text);
359380
ALTERTABLE test4 ALTER COLUMN f1SET STORAGE EXTERNAL;-- disable compression

0 commit comments

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