Merge pull request #2676 from cloudflare/yagiz/remove-raii

remove raiicontext and replace close() calls

src/workerd/api/node/zlib-util.c++

@@ -397,10 +397,8 @@ kj::Maybe<CompressionError> 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<kj::byte> input,
@@ -527,7 +523,7 @@ void ZlibUtil::CompressionStream<CompressionContext>::close() {
   }
   closed = true;
   JSG_ASSERT(initialized, Error, "Closing before initialized"_kj);
-  context()->close();
+  // Context is closed on the destructor of the CompressionContext.
 }
 
 template <typename CompressionContext>
@@ -655,12 +651,6 @@ BrotliEncoderContext::BrotliEncoderContext(ZlibMode _mode): BrotliContext(_mode)
   state = kj::disposeWith<BrotliEncoderDestroyInstance>(instance);
 }
 
-void BrotliEncoderContext::close() {
-  auto instance = BrotliEncoderCreateInstance(alloc_brotli, free_brotli, alloc_opaque_brotli);
-  state = kj::disposeWith<BrotliEncoderDestroyInstance>(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<BrotliDecoderDestroyInstance>(instance);
 }
 
-void BrotliDecoderContext::close() {
-  auto instance = BrotliDecoderCreateInstance(alloc_brotli, free_brotli, alloc_opaque_brotli);
-  state = kj::disposeWith<BrotliDecoderDestroyInstance>(kj::mv(instance));
-  mode = ZlibMode::NONE;
-}
-
 kj::Maybe<CompressionError> 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<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 +845,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 +894,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);

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<kj::byte> input,
       uint32_t inputLength,
       kj::ArrayPtr<kj::byte> 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<CompressionError> initialize(
@@ -264,13 +263,13 @@ 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);
 
-  void close();
+  KJ_DISALLOW_COPY_AND_MOVE(BrotliDecoderContext);
+
   // Equivalent to Node.js' `DoThreadPoolWork` implementation.
   void work();
   kj::Maybe<CompressionError> 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);