From 18c0210e2692ae2b8c17267d7683d4fe8eab2923 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Mon, 9 Sep 2024 14:25:28 -0400 Subject: [PATCH] remove raiicontext and replace close() calls --- src/workerd/api/node/zlib-util.c++ | 36 ++++-------------------------- src/workerd/api/node/zlib-util.h | 20 ++++++++--------- 2 files changed, 14 insertions(+), 42 deletions(-) diff --git a/src/workerd/api/node/zlib-util.c++ b/src/workerd/api/node/zlib-util.c++ index 3493b1a283a..764527ac862 100644 --- a/src/workerd/api/node/zlib-util.c++ +++ b/src/workerd/api/node/zlib-util.c++ @@ -397,10 +397,8 @@ kj::Maybe ZlibContext::setParams(int _level, int _strategy) { return kj::none; } -void ZlibContext::close() { +ZlibContext::~ZlibContext() noexcept(false) { if (!initialized) { - dictionary.clear(); - mode = ZlibMode::NONE; return; } @@ -423,8 +421,6 @@ void ZlibContext::close() { JSG_REQUIRE( status == Z_OK || status == Z_DATA_ERROR, Error, "Uncaught error on closing zlib stream"); - mode = ZlibMode::NONE; - dictionary.clear(); } void ZlibContext::setBuffers(kj::ArrayPtr input, @@ -527,7 +523,7 @@ void ZlibUtil::CompressionStream::close() { } closed = true; JSG_ASSERT(initialized, Error, "Closing before initialized"_kj); - context()->close(); + // Context is closed on the destructor of the CompressionContext. } template @@ -655,12 +651,6 @@ BrotliEncoderContext::BrotliEncoderContext(ZlibMode _mode): BrotliContext(_mode) state = kj::disposeWith(instance); } -void BrotliEncoderContext::close() { - auto instance = BrotliEncoderCreateInstance(alloc_brotli, free_brotli, alloc_opaque_brotli); - state = kj::disposeWith(kj::mv(instance)); - mode = ZlibMode::NONE; -} - void BrotliEncoderContext::work() { JSG_REQUIRE(mode == ZlibMode::BROTLI_ENCODE, Error, "Mode should be BROTLI_ENCODE"_kj); JSG_REQUIRE_NONNULL(state.get(), Error, "State should not be empty"_kj); @@ -713,12 +703,6 @@ BrotliDecoderContext::BrotliDecoderContext(ZlibMode _mode): BrotliContext(_mode) state = kj::disposeWith(instance); } -void BrotliDecoderContext::close() { - auto instance = BrotliDecoderCreateInstance(alloc_brotli, free_brotli, alloc_opaque_brotli); - state = kj::disposeWith(kj::mv(instance)); - mode = ZlibMode::NONE; -} - kj::Maybe BrotliDecoderContext::initialize( brotli_alloc_func init_alloc_func, brotli_free_func init_free_func, void* init_opaque_func) { alloc_brotli = init_alloc_func; @@ -840,18 +824,6 @@ void ZlibUtil::CompressionStream::FreeForZlib(void* data, vo JSG_REQUIRE(ctx->allocations.erase(real_pointer), Error, "Zlib allocation should exist"_kj); } namespace { -// A RAII wrapper around a compression context class -// TODO(soon): See if this functionality can just be embedded into each CompressionContext -template -class ContextRAII: public CompressionContext { -public: - using CompressionContext::CompressionContext; - - ~ContextRAII() { - static_cast(this)->close(); - } -}; - template static kj::Array syncProcessBuffer(Context& ctx, GrowableBuffer& result) { do { @@ -873,7 +845,7 @@ static kj::Array syncProcessBuffer(Context& ctx, GrowableBuffer& resul kj::Array ZlibUtil::zlibSync( ZlibUtil::InputSource data, ZlibContext::Options opts, ZlibModeValue mode) { - ContextRAII ctx(static_cast(mode)); + ZlibContext ctx(static_cast(mode)); auto chunkSize = opts.chunkSize.orDefault(ZLIB_PERFORMANT_CHUNK_SIZE); auto maxOutputLength = opts.maxOutputLength.orDefault(Z_MAX_CHUNK); @@ -922,7 +894,7 @@ void ZlibUtil::zlibWithCallback(jsg::Lock& js, template kj::Array ZlibUtil::brotliSync(InputSource data, BrotliContext::Options opts) { - ContextRAII ctx(Context::Mode); + Context ctx(Context::Mode); auto chunkSize = opts.chunkSize.orDefault(ZLIB_PERFORMANT_CHUNK_SIZE); auto maxOutputLength = opts.maxOutputLength.orDefault(Z_MAX_CHUNK); diff --git a/src/workerd/api/node/zlib-util.h b/src/workerd/api/node/zlib-util.h index d617ee9126c..e9a18bb6a7c 100644 --- a/src/workerd/api/node/zlib-util.h +++ b/src/workerd/api/node/zlib-util.h @@ -90,15 +90,14 @@ struct CompressionError { int err; }; -// TODO(soon): See if RAII support can be added directly to this class, and we can mark it final -class ZlibContext { +class ZlibContext final { public: explicit ZlibContext(ZlibMode _mode): mode(_mode) {} ZlibContext() = default; + ~ZlibContext() noexcept(false); - KJ_DISALLOW_COPY(ZlibContext); + KJ_DISALLOW_COPY_AND_MOVE(ZlibContext); - void close(); void setBuffers(kj::ArrayPtr input, uint32_t inputLength, kj::ArrayPtr output, @@ -244,13 +243,13 @@ class BrotliContext { void* alloc_opaque_brotli = nullptr; }; -// TODO(soon): See if RAII support can be added directly to this class, and we can mark it final -class BrotliEncoderContext: public BrotliContext { +class BrotliEncoderContext final: public BrotliContext { public: static const ZlibMode Mode = ZlibMode::BROTLI_ENCODE; explicit BrotliEncoderContext(ZlibMode _mode); - void close(); + KJ_DISALLOW_COPY_AND_MOVE(BrotliEncoderContext); + // Equivalent to Node.js' `DoThreadPoolWork` implementation. void work(); kj::Maybe initialize( @@ -264,13 +263,13 @@ class BrotliEncoderContext: public BrotliContext { kj::Own state; }; -// TODO(soon): See if RAII support can be added directly to this class, and we can mark it final -class BrotliDecoderContext: public BrotliContext { +class BrotliDecoderContext final: public BrotliContext { public: static const ZlibMode Mode = ZlibMode::BROTLI_DECODE; explicit BrotliDecoderContext(ZlibMode _mode); - void close(); + KJ_DISALLOW_COPY_AND_MOVE(BrotliDecoderContext); + // Equivalent to Node.js' `DoThreadPoolWork` implementation. void work(); kj::Maybe initialize( @@ -297,6 +296,7 @@ class ZlibUtil final: public jsg::Object { public: explicit CompressionStream(ZlibMode _mode): context_(_mode) {} CompressionStream() = default; + // TODO(soon): Find a way to add noexcept(false) to this destructor. ~CompressionStream(); KJ_DISALLOW_COPY_AND_MOVE(CompressionStream);