postgres.git / summary / log / commit / refs

commit    41479f34dda93dc462cd0794835798444ae6daa9
Author:   Stephen Frost <sfrost@snowman.net>
Date:     Mon Jan 12 15:13:18 2015 +0000

    Skip dead backends in MinimumActiveBackends
    
    Back in ed0b409, PGPROC was split and moved to static variables in
    procarray.c, with procs in ProcArrayStruct replaced by an array of
    integers representing process numbers (pgprocnos), with -1 indicating a
    dead process which has yet to be removed.  Access to procArray is
    generally done under ProcArrayLock and therefore most code does not have
    to concern itself with -1 entries.
    
    However, MinimumActiveBackends intentionally does not take
    ProcArrayLock, which means it has to be extra careful when accessing
    procArray.  Prior to ed0b409, this was handled by checking for a NULL
    in the pointer array, but that check was no longer valid after the
    split.  Coverity pointed out that the check could never happen and so
    it was removed in 5592eba.  That didn't make anything worse, but it
    didn't fix the issue either.
    
    The correct fix is to check for pgprocno == -1 and skip over that entry
    if it is encountered.
    
    Back-patch to 9.2, since there can be attempts to access the arrays
    prior to their start otherwise.  Note that the changes prior to 9.4 will
    look a bit different due to the change in 5592eba.
    
    Note that MinimumActiveBackends only returns a bool for heuristic
    purposes and any pre-array accesses are strictly read-only and so there
    is no security implication and the lack of fields complaints indicates
    it's very unlikely to run into issues due to this.
    
    Pointed out by Noah.


src/backend/storage/ipc/procarray.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index 85742710d32..2f22937e693 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -2258,9 +2258,8 @@ MinimumActiveBackends(int min) * free list and are recycled. Its contents are nonsense in that case, * but that's acceptable for this function. */ - if (proc == NULL) - continue; - + if (pgprocno == -1) + continue; /* do not count deleted entries */ if (proc == MyProc) continue; /* do not count myself */ if (pgxact->xid == InvalidTransactionId) [parent: 5ca4e444cb22]