Add missing deflateEnd() for server-side gzip base backups

The gzip basebackup sink called deflateInit2() in begin_archive() but
never called deflateEnd(), leaking zlib's internal compression state
(~256KB per archive) until the memory context of the base backup is
destroyed.

The code tree has already a matching deflateEnd() call for each
deflateInit[2]() call (pgrypto, etc.), except for the file touched in
this commit, so this brings more consistency for all the compression
methods.  The server-side LZ4 and zstd implementations require a
dedicated cleanup callback as they allocate their state outside the
context of a palloc().

As currently used, deflateInit2() is called once per tablespace in a
single backup.  Memory would slightly bloat only when dealing with many
tablespaces at once, not across multiple base backups so this is not
worth a backpatch.  This change could matter for future uses of this
code.

zlib allows the definition of memory allocation and free callbacks in
the z_stream object given to a deflateInit[2]().  The base backup
backend code relies on palloc() for the allocations and deflateEnd()
internally only cleans up memory (no fd allocation for example).

Author: Jianghua Yang <yjhjstz@gmail.com>
Discussion: https://postgr.es/m/CAAZLFmQNJ0QNArpWEOZXwv=vbumcWKEHz-b1me5gBqRqG67EwQ@mail.gmail.com

diff --git a/src/backend/backup/basebackup_gzip.c b/src/backend/backup/basebackup_gzip.c
index 1ba25015ab7193feea3d97513c277ba2e6d2dd13..c5e4c4143e80cc6bd289fb619ef9022f7cfecad7 100644 (file)
--- a/src/backend/backup/basebackup_gzip.c
+++ b/src/backend/backup/basebackup_gzip.c
@@ -32,6 +32,9 @@ typedef struct bbsink_gzip
 
        /* Number of bytes staged in output buffer. */
        size_t          bytes_written;
+
+       /* Has the zstream been initialized? */
+       bool            zstream_initialized;
 } bbsink_gzip;
 
 static void bbsink_gzip_begin_backup(bbsink *sink);
@@ -39,6 +42,7 @@ static void bbsink_gzip_begin_archive(bbsink *sink, const char *archive_name);
 static void bbsink_gzip_archive_contents(bbsink *sink, size_t len);
 static void bbsink_gzip_manifest_contents(bbsink *sink, size_t len);
 static void bbsink_gzip_end_archive(bbsink *sink);
+static void bbsink_gzip_cleanup(bbsink *sink);
 static void *gzip_palloc(void *opaque, unsigned items, unsigned size);
 static void gzip_pfree(void *opaque, void *address);
 
@@ -51,7 +55,7 @@ static const bbsink_ops bbsink_gzip_ops = {
        .manifest_contents = bbsink_gzip_manifest_contents,
        .end_manifest = bbsink_forward_end_manifest,
        .end_backup = bbsink_forward_end_backup,
-       .cleanup = bbsink_forward_cleanup
+       .cleanup = bbsink_gzip_cleanup
 };
 #endif
 
@@ -141,6 +145,7 @@ bbsink_gzip_begin_archive(bbsink *sink, const char *archive_name)
                ereport(ERROR,
                                errcode(ERRCODE_INTERNAL_ERROR),
                                errmsg("could not initialize compression library"));
+       mysink->zstream_initialized = true;
 
        /*
         * Add ".gz" to the archive name. Note that the pg_basebackup -z produces
@@ -266,6 +271,10 @@ bbsink_gzip_end_archive(bbsink *sink)
                mysink->bytes_written = 0;
        }
 
+       /* Release the compression resources. */
+       deflateEnd(zs);
+       mysink->zstream_initialized = false;
+
        /* Must also pass on the information that this archive has ended. */
        bbsink_forward_end_archive(sink);
 }
@@ -301,4 +310,20 @@ gzip_pfree(void *opaque, void *address)
        pfree(address);
 }
 
+/*
+ * In case the backup fails, make sure we free the compression context by
+ * calling deflateEnd() if needed to avoid a resource leak.
+ */
+static void
+bbsink_gzip_cleanup(bbsink *sink)
+{
+       bbsink_gzip *mysink = (bbsink_gzip *) sink;
+
+       if (mysink->zstream_initialized)
+       {
+               deflateEnd(&mysink->zstream);
+               mysink->zstream_initialized = false;
+       }
+}
+
 #endif