remove raiicontext and replace close() calls

cloudflare · Sep 9, 2024 · f890855 · f890855
1 parent 616dfc5
commit f890855
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 27 deletions.
diff --git a/src/workerd/api/node/zlib-util.c++ b/src/workerd/api/node/zlib-util.c++
@@ -397,7 +397,7 @@ kj::Maybe<CompressionError> ZlibContext::setParams(int _level, int _strategy) {
   return kj::none;
 }
 
-void ZlibContext::close() {
+ZlibContext::~ZlibContext() {
   if (!initialized) {
     dictionary.clear();
     mode = ZlibMode::NONE;
@@ -527,7 +527,7 @@ void ZlibUtil::CompressionStream<CompressionContext>::close() {
   }
   closed = true;
   JSG_ASSERT(initialized, Error, "Closing before initialized"_kj);
-  context()->close();
+  // Context is closed on the destructor.
 }
 
 template <typename CompressionContext>
@@ -655,7 +655,7 @@ BrotliEncoderContext::BrotliEncoderContext(ZlibMode _mode): BrotliContext(_mode)
   state = kj::disposeWith<BrotliEncoderDestroyInstance>(instance);
 }
 
-void BrotliEncoderContext::close() {
+BrotliEncoderContext::~BrotliEncoderContext() {
   auto instance = BrotliEncoderCreateInstance(alloc_brotli, free_brotli, alloc_opaque_brotli);
   state = kj::disposeWith<BrotliEncoderDestroyInstance>(kj::mv(instance));
   mode = ZlibMode::NONE;
@@ -713,7 +713,7 @@ BrotliDecoderContext::BrotliDecoderContext(ZlibMode _mode): BrotliContext(_mode)
   state = kj::disposeWith<BrotliDecoderDestroyInstance>(instance);
 }
 
-void BrotliDecoderContext::close() {
+BrotliDecoderContext::~BrotliDecoderContext() {
   auto instance = BrotliDecoderCreateInstance(alloc_brotli, free_brotli, alloc_opaque_brotli);
   state = kj::disposeWith<BrotliDecoderDestroyInstance>(kj::mv(instance));
   mode = ZlibMode::NONE;
@@ -840,18 +840,6 @@ void ZlibUtil::CompressionStream<CompressionContext>::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 <typename CompressionContext>
-class ContextRAII: public CompressionContext {
-public:
-  using CompressionContext::CompressionContext;
-
-  ~ContextRAII() {
-    static_cast<CompressionContext*>(this)->close();
-  }
-};
-
 template <typename Context>
 static kj::Array<kj::byte> syncProcessBuffer(Context& ctx, GrowableBuffer& result) {
   do {
@@ -873,7 +861,7 @@ static kj::Array<kj::byte> syncProcessBuffer(Context& ctx, GrowableBuffer& resul
 
 kj::Array<kj::byte> ZlibUtil::zlibSync(
     ZlibUtil::InputSource data, ZlibContext::Options opts, ZlibModeValue mode) {
-  ContextRAII<ZlibContext> ctx(static_cast<ZlibMode>(mode));
+  ZlibContext ctx(static_cast<ZlibMode>(mode));
 
   auto chunkSize = opts.chunkSize.orDefault(ZLIB_PERFORMANT_CHUNK_SIZE);
   auto maxOutputLength = opts.maxOutputLength.orDefault(Z_MAX_CHUNK);
@@ -922,7 +910,7 @@ void ZlibUtil::zlibWithCallback(jsg::Lock& js,
 
 template <typename Context>
 kj::Array<kj::byte> ZlibUtil::brotliSync(InputSource data, BrotliContext::Options opts) {
-  ContextRAII<Context> 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
@@ -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();
 
   KJ_DISALLOW_COPY(ZlibContext);
 
-  void close();
   void setBuffers(kj::ArrayPtr<kj::byte> input,
       uint32_t inputLength,
       kj::ArrayPtr<kj::byte> output,
@@ -244,13 +243,12 @@ 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);
+  ~BrotliEncoderContext();
 
-  void close();
   // Equivalent to Node.js' `DoThreadPoolWork` implementation.
   void work();
   kj::Maybe<CompressionError> initialize(
@@ -264,13 +262,12 @@ class BrotliEncoderContext: public BrotliContext {
   kj::Own<BrotliEncoderStateStruct> 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);
+  ~BrotliDecoderContext();
 
-  void close();
   // Equivalent to Node.js' `DoThreadPoolWork` implementation.
   void work();
   kj::Maybe<CompressionError> initialize(