Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove raiicontext and replace close() calls #2676

Merged
merged 1 commit into from
Sep 9, 2024
Merged

remove raiicontext and replace close() calls #2676

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 4 additions & 32 deletions src/workerd/api/node/zlib-util.c++
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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,
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
20 changes: 10 additions & 10 deletions src/workerd/api/node/zlib-util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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);

Expand Down