Fix data-corruption hazard in WAL-logged CREATE DATABASE.
authorTom Lane <tgl@sss.pgh.pa.us>
Sat, 6 Aug 2022 15:50:23 +0000 (11:50 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Sat, 6 Aug 2022 15:50:34 +0000 (11:50 -0400)
RelationCopyStorageUsingBuffer thought it could skip copying
empty pages, but of course that does not work at all, because
subsequent blocks will be out of place.

Also fix it to acquire share lock on the source buffer.  It *might*
be safe to not do that, but it's not very certain, and I don't think
this code deserves any benefit of the doubt.

Dilip Kumar, per complaint from me

Discussion: https://postgr.es/m/3679800.1659654066@sss.pgh.pa.us

src/backend/storage/buffer/bufmgr.c

index 6b30138ab32ed184994f5b17841441b35fd8cab6..8ef0436c521587bd99e1b495bb114ec18c1bb65f 100644 (file)
@@ -3741,23 +3741,19 @@ RelationCopyStorageUsingBuffer(Relation src, Relation dst, ForkNumber forkNum,
        srcBuf = ReadBufferWithoutRelcache(src->rd_locator, forkNum, blkno,
                                           RBM_NORMAL, bstrategy_src,
                                           permanent);
+       LockBuffer(srcBuf, BUFFER_LOCK_SHARE);
        srcPage = BufferGetPage(srcBuf);
-       if (PageIsNew(srcPage) || PageIsEmpty(srcPage))
-       {
-           ReleaseBuffer(srcBuf);
-           continue;
-       }
 
        /* Use P_NEW to extend the destination relation. */
        dstBuf = ReadBufferWithoutRelcache(dst->rd_locator, forkNum, P_NEW,
                                           RBM_NORMAL, bstrategy_dst,
                                           permanent);
        LockBuffer(dstBuf, BUFFER_LOCK_EXCLUSIVE);
+       dstPage = BufferGetPage(dstBuf);
 
        START_CRIT_SECTION();
 
        /* Copy page data from the source to the destination. */
-       dstPage = BufferGetPage(dstBuf);
        memcpy(dstPage, srcPage, BLCKSZ);
        MarkBufferDirty(dstBuf);
 
@@ -3768,7 +3764,7 @@ RelationCopyStorageUsingBuffer(Relation src, Relation dst, ForkNumber forkNum,
        END_CRIT_SECTION();
 
        UnlockReleaseBuffer(dstBuf);
-       ReleaseBuffer(srcBuf);
+       UnlockReleaseBuffer(srcBuf);
    }
 }