• Tom Lane's avatar
    Fix memory leak and other bugs in ginPlaceToPage() & subroutines. · bde361fe
    Tom Lane authored
    Commit 36a35c55 turned the interface between ginPlaceToPage and
    its subroutines in gindatapage.c and ginentrypage.c into a royal mess:
    page-update critical sections were started in one place and finished in
    another place not even in the same file, and the very same subroutine
    might return having started a critical section or not.  Subsequent patches
    band-aided over some of the problems with this design by making things
    even messier.
    
    One user-visible resulting problem is memory leaks caused by the need for
    the subroutines to allocate storage that would survive until ginPlaceToPage
    calls XLogInsert (as reported by Julien Rouhaud).  This would not typically
    be noticeable during retail index updates.  It could be visible in a GIN
    index build, in the form of memory consumption swelling to several times
    the commanded maintenance_work_mem.
    
    Another rather nasty problem is that in the internal-page-splitting code
    path, we would clear the child page's GIN_INCOMPLETE_SPLIT flag well before
    entering the critical section that it's supposed to be cleared in; a
    failure in between would leave the index in a corrupt state.  There were
    also assorted coding-rule violations with little immediate consequence but
    possible long-term hazards, such as beginning an XLogInsert sequence before
    entering a critical section, or calling elog(DEBUG) inside a critical
    section.
    
    To fix, redefine the API between ginPlaceToPage() and its subroutines
    by splitting the subroutines into two parts.  The "beginPlaceToPage"
    subroutine does what can be done outside a critical section, including
    full computation of the result pages into temporary storage when we're
    going to split the target page.  The "execPlaceToPage" subroutine is called
    within a critical section established by ginPlaceToPage(), and it handles
    the actual page update in the non-split code path.  The critical section,
    as well as the XLOG insertion call sequence, are both now always started
    and finished in ginPlaceToPage().  Also, make ginPlaceToPage() create and
    work in a short-lived memory context to eliminate the leakage problem.
    (Since a short-lived memory context had been getting created in the most
    common code path in the subroutines, this shouldn't cause any noticeable
    performance penalty; we're just moving the overhead up one call level.)
    
    In passing, fix a bunch of comments that had gone unmaintained throughout
    all this klugery.
    
    Report: <571276DD.5050303@dalibo.com>
    bde361fe
ginentrypage.c 19.5 KB