mirror of
https://github.com/postgres/postgres.git
synced 2025-04-27 22:56:53 +03:00
Avoid using a fake relcache entry to own an SmgrRelation.
If an error occurs before we close the fake relcache entry, the the fake relcache entry will be destroyed by the SmgrRelation will survive until end of transaction. Its smgr_owner pointer ends up pointing to already-freed memory. The original reason for using a fake relcache entry here was to try to avoid reusing an SMgrRelation across a relevant invalidation. To avoid that problem, just call smgropen() again each time we need a reference to it. Hopefully someday we will come up with a more elegant approach, but accessing uninitialized memory is bad so let's do this for now. Dilip Kumar, reviewed by Andres Freund and Tom Lane. Report by Justin Pryzby. Discussion: http://postgr.es/m/20220802175043.GA13682@telsasoft.com Discussion: http://postgr.es/m/CAFiTN-vSFeE6_W9z698XNtFROOA_nSqUXWqLcG0emob_kJ+dEQ@mail.gmail.com
This commit is contained in:
Robert Haas
parent
3d895bc846
commit
76733b399c
@ -258,8 +258,8 @@ ScanSourceDatabasePgClass(Oid tbid, Oid dbid, char *srcpath)
|
||||
Page page;
|
||||
List *rlocatorlist = NIL;
|
||||
LockRelId relid;
|
||||
Relation rel;
|
||||
Snapshot snapshot;
|
||||
SMgrRelation smgr;
|
||||
BufferAccessStrategy bstrategy;
|
||||
|
||||
/* Get pg_class relfilenumber. */
|
||||
@ -276,16 +276,9 @@ ScanSourceDatabasePgClass(Oid tbid, Oid dbid, char *srcpath)
|
||||
rlocator.dbOid = dbid;
|
||||
rlocator.relNumber = relfilenumber;
|
||||
|
||||
/*
|
||||
* We can't use a real relcache entry for a relation in some other
|
||||
* database, but since we're only going to access the fields related to
|
||||
* physical storage, a fake one is good enough. If we didn't do this and
|
||||
* used the smgr layer directly, we would have to worry about
|
||||
* invalidations.
|
||||
*/
|
||||
rel = CreateFakeRelcacheEntry(rlocator);
|
||||
nblocks = smgrnblocks(RelationGetSmgr(rel), MAIN_FORKNUM);
|
||||
FreeFakeRelcacheEntry(rel);
|
||||
smgr = smgropen(rlocator, InvalidBackendId);
|
||||
nblocks = smgrnblocks(smgr, MAIN_FORKNUM);
|
||||
smgrclose(smgr);
|
||||
|
||||
/* Use a buffer access strategy since this is a bulk read operation. */
|
||||
bstrategy = GetAccessStrategy(BAS_BULKREAD);
|
||||
|
||||
@ -487,9 +487,9 @@ static void FindAndDropRelationBuffers(RelFileLocator rlocator,
|
||||
ForkNumber forkNum,
|
||||
BlockNumber nForkBlock,
|
||||
BlockNumber firstDelBlock);
|
||||
static void RelationCopyStorageUsingBuffer(Relation src, Relation dst,
|
||||
ForkNumber forkNum,
|
||||
bool isunlogged);
|
||||
static void RelationCopyStorageUsingBuffer(RelFileLocator srclocator,
|
||||
RelFileLocator dstlocator,
|
||||
ForkNumber forkNum, bool permanent);
|
||||
static void AtProcExit_Buffers(int code, Datum arg);
|
||||
static void CheckForBufferLeaks(void);
|
||||
static int rlocator_comparator(const void *p1, const void *p2);
|
||||
@ -3701,8 +3701,9 @@ FlushRelationsAllBuffers(SMgrRelation *smgrs, int nrels)
|
||||
* --------------------------------------------------------------------
|
||||
*/
|
||||
static void
|
||||
RelationCopyStorageUsingBuffer(Relation src, Relation dst, ForkNumber forkNum,
|
||||
bool permanent)
|
||||
RelationCopyStorageUsingBuffer(RelFileLocator srclocator,
|
||||
RelFileLocator dstlocator,
|
||||
ForkNumber forkNum, bool permanent)
|
||||
{
|
||||
Buffer srcBuf;
|
||||
Buffer dstBuf;
|
||||
@ -3722,7 +3723,8 @@ RelationCopyStorageUsingBuffer(Relation src, Relation dst, ForkNumber forkNum,
|
||||
use_wal = XLogIsNeeded() && (permanent || forkNum == INIT_FORKNUM);
|
||||
|
||||
/* Get number of blocks in the source relation. */
|
||||
nblocks = smgrnblocks(RelationGetSmgr(src), forkNum);
|
||||
nblocks = smgrnblocks(smgropen(srclocator, InvalidBackendId),
|
||||
forkNum);
|
||||
|
||||
/* Nothing to copy; just return. */
|
||||
if (nblocks == 0)
|
||||
@ -3738,14 +3740,14 @@ RelationCopyStorageUsingBuffer(Relation src, Relation dst, ForkNumber forkNum,
|
||||
CHECK_FOR_INTERRUPTS();
|
||||
|
||||
/* Read block from source relation. */
|
||||
srcBuf = ReadBufferWithoutRelcache(src->rd_locator, forkNum, blkno,
|
||||
srcBuf = ReadBufferWithoutRelcache(srclocator, forkNum, blkno,
|
||||
RBM_NORMAL, bstrategy_src,
|
||||
permanent);
|
||||
LockBuffer(srcBuf, BUFFER_LOCK_SHARE);
|
||||
srcPage = BufferGetPage(srcBuf);
|
||||
|
||||
/* Use P_NEW to extend the destination relation. */
|
||||
dstBuf = ReadBufferWithoutRelcache(dst->rd_locator, forkNum, P_NEW,
|
||||
dstBuf = ReadBufferWithoutRelcache(dstlocator, forkNum, P_NEW,
|
||||
RBM_NORMAL, bstrategy_dst,
|
||||
permanent);
|
||||
LockBuffer(dstBuf, BUFFER_LOCK_EXCLUSIVE);
|
||||
@ -3783,24 +3785,13 @@ void
|
||||
CreateAndCopyRelationData(RelFileLocator src_rlocator,
|
||||
RelFileLocator dst_rlocator, bool permanent)
|
||||
{
|
||||
Relation src_rel;
|
||||
Relation dst_rel;
|
||||
RelFileLocatorBackend rlocator;
|
||||
char relpersistence;
|
||||
|
||||
/* Set the relpersistence. */
|
||||
relpersistence = permanent ?
|
||||
RELPERSISTENCE_PERMANENT : RELPERSISTENCE_UNLOGGED;
|
||||
|
||||
/*
|
||||
* We can't use a real relcache entry for a relation in some other
|
||||
* database, but since we're only going to access the fields related to
|
||||
* physical storage, a fake one is good enough. If we didn't do this and
|
||||
* used the smgr layer directly, we would have to worry about
|
||||
* invalidations.
|
||||
*/
|
||||
src_rel = CreateFakeRelcacheEntry(src_rlocator);
|
||||
dst_rel = CreateFakeRelcacheEntry(dst_rlocator);
|
||||
|
||||
/*
|
||||
* Create and copy all forks of the relation. During create database we
|
||||
* have a separate cleanup mechanism which deletes complete database
|
||||
@ -3810,15 +3801,16 @@ CreateAndCopyRelationData(RelFileLocator src_rlocator,
|
||||
RelationCreateStorage(dst_rlocator, relpersistence, false);
|
||||
|
||||
/* copy main fork. */
|
||||
RelationCopyStorageUsingBuffer(src_rel, dst_rel, MAIN_FORKNUM, permanent);
|
||||
RelationCopyStorageUsingBuffer(src_rlocator, dst_rlocator, MAIN_FORKNUM,
|
||||
permanent);
|
||||
|
||||
/* copy those extra forks that exist */
|
||||
for (ForkNumber forkNum = MAIN_FORKNUM + 1;
|
||||
forkNum <= MAX_FORKNUM; forkNum++)
|
||||
{
|
||||
if (smgrexists(RelationGetSmgr(src_rel), forkNum))
|
||||
if (smgrexists(smgropen(src_rlocator, InvalidBackendId), forkNum))
|
||||
{
|
||||
smgrcreate(RelationGetSmgr(dst_rel), forkNum, false);
|
||||
smgrcreate(smgropen(dst_rlocator, InvalidBackendId), forkNum, false);
|
||||
|
||||
/*
|
||||
* WAL log creation if the relation is persistent, or this is the
|
||||
@ -3828,14 +3820,19 @@ CreateAndCopyRelationData(RelFileLocator src_rlocator,
|
||||
log_smgrcreate(&dst_rlocator, forkNum);
|
||||
|
||||
/* Copy a fork's data, block by block. */
|
||||
RelationCopyStorageUsingBuffer(src_rel, dst_rel, forkNum,
|
||||
RelationCopyStorageUsingBuffer(src_rlocator, dst_rlocator, forkNum,
|
||||
permanent);
|
||||
}
|
||||
}
|
||||
|
||||
/* Release fake relcache entries. */
|
||||
FreeFakeRelcacheEntry(src_rel);
|
||||
FreeFakeRelcacheEntry(dst_rel);
|
||||
/* close source and destination smgr if exists. */
|
||||
rlocator.backend = InvalidBackendId;
|
||||
|
||||
rlocator.locator = src_rlocator;
|
||||
smgrcloserellocator(rlocator);
|
||||
|
||||
rlocator.locator = dst_rlocator;
|
||||
smgrcloserellocator(rlocator);
|
||||
}
|
||||
|
||||
/* ---------------------------------------------------------------------
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user