public inbox for [email protected]
help / color / mirror / Atom feedFrom: Jianghua Yang <[email protected]>
To: [email protected]
Subject: basebackup: add missing deflateEnd() in gzip compression sink
Date: Fri, 20 Mar 2026 10:02:44 -0700
Message-ID: <CAAZLFmQNJ0QNArpWEOZXwv=vbumcWKEHz-b1me5gBqRqG67EwQ@mail.gmail.com> (raw)
Hi hackers,
While reviewing the backup compression backends, I noticed that the gzip
sink (basebackup_gzip.c) never calls deflateEnd(), unlike the lz4 and
zstd sinks which properly free their compression contexts.
Specifically:
- lz4 calls LZ4F_freeCompressionContext() in both end_archive and a
dedicated bbsink_lz4_cleanup() callback.
- zstd calls ZSTD_freeCCtx() in both end_archive and a dedicated
bbsink_zstd_cleanup() callback.
- gzip calls deflateInit2() in begin_archive but never calls
deflateEnd(). Its cleanup callback is bbsink_forward_cleanup,
which just forwards to the next sink without releasing the zlib
context.
This means each archive (one per tablespace) leaks ~256KB of zlib
internal state until the backup's memory context is destroyed. On the
ERROR path, the zlib context is not released at all since there is no
gzip-specific cleanup callback.
The attached patch adds deflateEnd() at the end of
bbsink_gzip_end_archive() for the normal path, and introduces a new
bbsink_gzip_cleanup() for the error path. The new cleanup callback
follows the exact same call chain as bbsink_lz4_cleanup and
bbsink_zstd_cleanup:
PG_FINALLY (basebackup.c:1063)
-> bbsink_cleanup(sink) (basebackup.c:1065)
-> sink->bbs_ops->cleanup(sink) (basebackup_sink.h:269)
-> bbsink_gzip_cleanup() -- now calls deflateEnd()
Previously the gzip slot in this chain pointed to
bbsink_forward_cleanup, which just forwarded to the next sink
without doing any gzip-specific resource release.
Since z_stream is an embedded struct (not a pointer like LZ4F_cctx or
ZSTD_CCtx), a bool flag "zstream_initialized" is used to track whether
deflateEnd() needs to be called.
Tested with pg_basebackup using gzip compression at levels 1, 5
(default), and 9, including server-side compression. All produced
valid .tar.gz files that pass gzip -t integrity checks and restore
correctly.
Regards,
Jianghua Yang
Attachments:
[application/octet-stream] 0001-basebackup-add-missing-deflateEnd-calls-in-gzip-comp.patch (3.3K, 3-0001-basebackup-add-missing-deflateEnd-calls-in-gzip-comp.patch)
download | inline diff:
From 5d1edeceafd422072c17026dfd1f862a19feb792 Mon Sep 17 00:00:00 2001
From: Jianghua Yang <[email protected]>
Date: Fri, 20 Mar 2026 09:57:17 -0700
Subject: [PATCH] basebackup: add missing deflateEnd() calls in gzip
compression sink
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 was destroyed. It also
lacked a dedicated cleanup callback, so on ERROR the zlib context
was not released at all.
Add deflateEnd() at the end of bbsink_gzip_end_archive() for the
normal path, and add a new bbsink_gzip_cleanup() function for the
error path. This brings the gzip sink in line with the lz4 and zstd
sinks, which already properly free their compression contexts in both
end_archive and cleanup callbacks.
Author: Jianghua Yang <[email protected]>
---
src/backend/backup/basebackup_gzip.c | 27 ++++++++++++++++++++++++++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/src/backend/backup/basebackup_gzip.c b/src/backend/backup/basebackup_gzip.c
index 1ba25015ab7..c264a833429 100644
--- 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 with deflateInit2? */
+ 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
--
2.50.1 (Apple Git-155)
view thread (6+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: basebackup: add missing deflateEnd() in gzip compression sink
In-Reply-To: <CAAZLFmQNJ0QNArpWEOZXwv=vbumcWKEHz-b1me5gBqRqG67EwQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox