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

Commit7b5d4c2

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 parent2d44dee commit7b5d4c2

File tree

8 files changed

+93
-8
lines changed

8 files changed

+93
-8
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4863,6 +4863,7 @@ CommitSubTransaction(void)
48634863
AfterTriggerEndSubXact(true);
48644864
AtSubCommit_Portals(s->subTransactionId,
48654865
s->parent->subTransactionId,
4866+
s->parent->nestingLevel,
48664867
s->parent->curTransactionOwner);
48674868
AtEOSubXact_LargeObject(true,s->subTransactionId,
48684869
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
/*
@@ -1136,9 +1138,15 @@ PortalRunUtility(Portal portal, PlannedStmt *pstmt,
11361138
snapshot=RegisterSnapshot(snapshot);
11371139
portal->holdSnapshot=snapshot;
11381140
}
1139-
/* In any case, make the snapshot active and remember it in portal */
1140-
PushActiveSnapshot(snapshot);
1141-
/* PushActiveSnapshot might have copied the snapshot */
1141+
1142+
/*
1143+
* In any case, make the snapshot active and remember it in portal.
1144+
* Because the portal now references the snapshot, we must tell
1145+
* snapmgr.c that the snapshot belongs to the portal's transaction
1146+
* level, else we risk portalSnapshot becoming a dangling pointer.
1147+
*/
1148+
PushActiveSnapshotWithLevel(snapshot,portal->createLevel);
1149+
/* PushActiveSnapshotWithLevel might have copied the snapshot */
11421150
portal->portalSnapshot=GetActiveSnapshot();
11431151
}
11441152
else
@@ -1784,8 +1792,13 @@ EnsurePortalSnapshotExists(void)
17841792
elog(ERROR,"cannot execute SQL without an outer snapshot or portal");
17851793
Assert(portal->portalSnapshot==NULL);
17861794

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

‎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
@@ -678,10 +678,25 @@ FreeSnapshot(Snapshot snapshot)
678678
*/
679679
void
680680
PushActiveSnapshot(Snapshotsnap)
681+
{
682+
PushActiveSnapshotWithLevel(snap,GetCurrentTransactionNestLevel());
683+
}
684+
685+
/*
686+
* PushActiveSnapshotWithLevel
687+
*Set the given snapshot as the current active snapshot
688+
*
689+
* Same as PushActiveSnapshot except that caller can specify the
690+
* transaction nesting level that "owns" the snapshot. This level
691+
* must not be deeper than the current top of the snapshot stack.
692+
*/
693+
void
694+
PushActiveSnapshotWithLevel(Snapshotsnap,intsnap_level)
681695
{
682696
ActiveSnapshotElt*newactive;
683697

684698
Assert(snap!=InvalidSnapshot);
699+
Assert(ActiveSnapshot==NULL||snap_level >=ActiveSnapshot->as_level);
685700

686701
newactive=MemoryContextAlloc(TopTransactionContext,sizeof(ActiveSnapshotElt));
687702

@@ -695,7 +710,7 @@ PushActiveSnapshot(Snapshot snap)
695710
newactive->as_snap=snap;
696711

697712
newactive->as_next=ActiveSnapshot;
698-
newactive->as_level=GetCurrentTransactionNestLevel();
713+
newactive->as_level=snap_level;
699714

700715
newactive->as_snap->active_count++;
701716

‎src/include/utils/portal.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ typedef struct PortalData
130130
*/
131131
SubTransactionIdcreateSubid;/* the creating subxact */
132132
SubTransactionIdactiveSubid;/* the last subxact with activity */
133+
intcreateLevel;/* creating subxact's nesting level */
133134

134135
/* The query or queries the portal will execute */
135136
constchar*sourceText;/* text of query (as of 8.4, never NULL) */
@@ -219,6 +220,7 @@ extern void AtCleanup_Portals(void);
219220
externvoidPortalErrorCleanup(void);
220221
externvoidAtSubCommit_Portals(SubTransactionIdmySubid,
221222
SubTransactionIdparentSubid,
223+
intparentLevel,
222224
ResourceOwnerparentXactOwner);
223225
externvoidAtSubAbort_Portals(SubTransactionIdmySubid,
224226
SubTransactionIdparentSubid,

‎src/include/utils/snapmgr.h

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

116116
externvoidPushActiveSnapshot(Snapshotsnapshot);
117+
externvoidPushActiveSnapshotWithLevel(Snapshotsnapshot,intsnap_level);
117118
externvoidPushCopiedSnapshot(Snapshotsnapshot);
118119
externvoidUpdateActiveSnapshotCommandId(void);
119120
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