pgstat_heap() creates a BufferAccessStrategy and attaches it to a
HeapScanDesc.  It continues to use that strategy after calling heap_endscan(),
which frees the strategy.  This is only a risk when the table contains empty
pages at the end.  I get a crash in an assert-enabled build with this test
procedure, after disabling autovacuum:

-- session 1
create table t (c) as select * from generate_series(1,20000);
delete from t where c > 10000;
-- session 2
begin; lock table t in access share mode;
-- session 1
vacuum t;
-- restart PostgreSQL to clear shared buffers
-- session 3
select * from pgstattuple('t');

The simplest fix is to move the heap_endscan() call past the last use of the
strategy.  However, I don't think this function ought to be creating a
strategy explicitly.  It should use the one that initscan() creates, if any.

-- 
Noah Misch
EnterpriseDB                                 http://www.enterprisedb.com
diff --git a/contrib/pgstattuple/pgstattuple.c 
b/contrib/pgstattuple/pgstattuple.c
index edc603f..1007748 100644
--- a/contrib/pgstattuple/pgstattuple.c
+++ b/contrib/pgstattuple/pgstattuple.c
@@ -274,7 +274,6 @@ pgstat_heap(Relation rel, FunctionCallInfo fcinfo)
        BlockNumber tupblock;
        Buffer          buffer;
        pgstattuple_type stat = {0};
-       BufferAccessStrategy bstrategy;
        SnapshotData SnapshotDirty;
 
        /* Disable syncscan because we assume we scan from block zero upwards */
@@ -283,10 +282,6 @@ pgstat_heap(Relation rel, FunctionCallInfo fcinfo)
 
        nblocks = scan->rs_nblocks; /* # blocks to be scanned */
 
-       /* prepare access strategy for this table */
-       bstrategy = GetAccessStrategy(BAS_BULKREAD);
-       scan->rs_strategy = bstrategy;
-
        /* scan the relation */
        while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
        {
@@ -320,26 +315,28 @@ pgstat_heap(Relation rel, FunctionCallInfo fcinfo)
                {
                        CHECK_FOR_INTERRUPTS();
 
-                       buffer = ReadBufferExtended(rel, MAIN_FORKNUM, block, 
RBM_NORMAL, bstrategy);
+                       buffer = ReadBufferExtended(rel, MAIN_FORKNUM, block,
+                                                                               
RBM_NORMAL, scan->rs_strategy);
                        LockBuffer(buffer, BUFFER_LOCK_SHARE);
                        stat.free_space += PageGetHeapFreeSpace((Page) 
BufferGetPage(buffer));
                        UnlockReleaseBuffer(buffer);
                        block++;
                }
        }
-       heap_endscan(scan);
 
        while (block < nblocks)
        {
                CHECK_FOR_INTERRUPTS();
 
-               buffer = ReadBufferExtended(rel, MAIN_FORKNUM, block, 
RBM_NORMAL, bstrategy);
+               buffer = ReadBufferExtended(rel, MAIN_FORKNUM, block,
+                                                                       
RBM_NORMAL, scan->rs_strategy);
                LockBuffer(buffer, BUFFER_LOCK_SHARE);
                stat.free_space += PageGetHeapFreeSpace((Page) 
BufferGetPage(buffer));
                UnlockReleaseBuffer(buffer);
                block++;
        }
 
+       heap_endscan(scan);
        relation_close(rel, AccessShareLock);
 
        stat.table_len = (uint64) nblocks *BLCKSZ;
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to