Skip to content

Commit

Permalink
zlib: switch to lazy init for zlib streams
Browse files Browse the repository at this point in the history
PR-URL: #34048
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
puzpuzpuz authored and MylesBorins committed Jul 27, 2020
1 parent eecb92c commit 252f376
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 26 deletions.
22 changes: 10 additions & 12 deletions lib/zlib.js
Original file line number Diff line number Diff line change
Expand Up @@ -659,18 +659,13 @@ function Zlib(opts, mode) {
// to come up with a good solution that doesn't break our internal API,
// and with it all supported npm versions at the time of writing.
this._writeState = new Uint32Array(2);
if (!handle.init(windowBits,
level,
memLevel,
strategy,
this._writeState,
processCallback,
dictionary)) {
// TODO(addaleax): Sometimes we generate better error codes in C++ land,
// e.g. ERR_BROTLI_PARAM_SET_FAILED -- it's hard to access them with
// the current bindings setup, though.
throw new ERR_ZLIB_INITIALIZATION_FAILED();
}
handle.init(windowBits,
level,
memLevel,
strategy,
this._writeState,
processCallback,
dictionary);

ZlibBase.call(this, opts, mode, handle, zlibDefaultOpts);

Expand Down Expand Up @@ -816,6 +811,9 @@ function Brotli(opts, mode) {
new binding.BrotliDecoder(mode) : new binding.BrotliEncoder(mode);

this._writeState = new Uint32Array(2);
// TODO(addaleax): Sometimes we generate better error codes in C++ land,
// e.g. ERR_BROTLI_PARAM_SET_FAILED -- it's hard to access them with
// the current bindings setup, though.
if (!handle.init(brotliInitParamsArray,
this._writeState,
processCallback)) {
Expand Down
59 changes: 45 additions & 14 deletions src/node_zlib.cc
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,8 @@ class ZlibContext : public MemoryRetainer {
CompressionError ResetStream();

// Zlib-specific:
CompressionError Init(int level, int window_bits, int mem_level, int strategy,
std::vector<unsigned char>&& dictionary);
void Init(int level, int window_bits, int mem_level, int strategy,
std::vector<unsigned char>&& dictionary);
void SetAllocationFunctions(alloc_func alloc, free_func free, void* opaque);
CompressionError SetParams(int level, int strategy);

Expand All @@ -157,7 +157,10 @@ class ZlibContext : public MemoryRetainer {
private:
CompressionError ErrorForMessage(const char* message) const;
CompressionError SetDictionary();
bool InitZlib();

Mutex mutex_; // Protects zlib_init_done_.
bool zlib_init_done_ = false;
int err_ = 0;
int flush_ = 0;
int level_ = 0;
Expand Down Expand Up @@ -615,13 +618,8 @@ class ZlibStream : public CompressionStream<ZlibContext> {
AllocScope alloc_scope(wrap);
wrap->context()->SetAllocationFunctions(
AllocForZlib, FreeForZlib, static_cast<CompressionStream*>(wrap));
const CompressionError err =
wrap->context()->Init(level, window_bits, mem_level, strategy,
std::move(dictionary));
if (err.IsError())
wrap->EmitError(err);

return args.GetReturnValue().Set(!err.IsError());
wrap->context()->Init(level, window_bits, mem_level, strategy,
std::move(dictionary));
}

static void Params(const FunctionCallbackInfo<Value>& args) {
Expand Down Expand Up @@ -724,6 +722,15 @@ using BrotliEncoderStream = BrotliCompressionStream<BrotliEncoderContext>;
using BrotliDecoderStream = BrotliCompressionStream<BrotliDecoderContext>;

void ZlibContext::Close() {
{
Mutex::ScopedLock lock(mutex_);
if (!zlib_init_done_) {
dictionary_.clear();
mode_ = NONE;
return;
}
}

CHECK_LE(mode_, UNZIP);

int status = Z_OK;
Expand All @@ -742,6 +749,11 @@ void ZlibContext::Close() {


void ZlibContext::DoThreadPoolWork() {
bool first_init_call = InitZlib();
if (first_init_call && err_ != Z_OK) {
return;
}

const Bytef* next_expected_header_byte = nullptr;

// If the avail_out is left at 0, then it means that it ran out
Expand Down Expand Up @@ -897,6 +909,11 @@ CompressionError ZlibContext::GetErrorInfo() const {


CompressionError ZlibContext::ResetStream() {
bool first_init_call = InitZlib();
if (first_init_call && err_ != Z_OK) {
return ErrorForMessage("Failed to init stream before reset");
}

err_ = Z_OK;

switch (mode_) {
Expand Down Expand Up @@ -930,7 +947,7 @@ void ZlibContext::SetAllocationFunctions(alloc_func alloc,
}


CompressionError ZlibContext::Init(
void ZlibContext::Init(
int level, int window_bits, int mem_level, int strategy,
std::vector<unsigned char>&& dictionary) {
if (!((window_bits == 0) &&
Expand Down Expand Up @@ -974,6 +991,15 @@ CompressionError ZlibContext::Init(
window_bits_ *= -1;
}

dictionary_ = std::move(dictionary);
}

bool ZlibContext::InitZlib() {
Mutex::ScopedLock lock(mutex_);
if (zlib_init_done_) {
return false;
}

switch (mode_) {
case DEFLATE:
case GZIP:
Expand All @@ -995,15 +1021,15 @@ CompressionError ZlibContext::Init(
UNREACHABLE();
}

dictionary_ = std::move(dictionary);

if (err_ != Z_OK) {
dictionary_.clear();
mode_ = NONE;
return ErrorForMessage("zlib error");
return true;
}

return SetDictionary();
SetDictionary();
zlib_init_done_ = true;
return true;
}


Expand Down Expand Up @@ -1040,6 +1066,11 @@ CompressionError ZlibContext::SetDictionary() {


CompressionError ZlibContext::SetParams(int level, int strategy) {
bool first_init_call = InitZlib();
if (first_init_call && err_ != Z_OK) {
return ErrorForMessage("Failed to init stream before set parameters");
}

err_ = Z_OK;

switch (mode_) {
Expand Down
37 changes: 37 additions & 0 deletions test/parallel/test-zlib-reset-before-write.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const zlib = require('zlib');

// Tests that zlib streams support .reset() and .params()
// before the first write. That is important to ensure that
// lazy init of zlib native library handles these cases.

for (const fn of [
(z, cb) => {
z.reset();
cb();
},
(z, cb) => z.params(0, zlib.constants.Z_DEFAULT_STRATEGY, cb)
]) {
const deflate = zlib.createDeflate();
const inflate = zlib.createInflate();

deflate.pipe(inflate);

const output = [];
inflate
.on('error', (err) => {
assert.ifError(err);
})
.on('data', (chunk) => output.push(chunk))
.on('end', common.mustCall(
() => assert.deepStrictEqual(Buffer.concat(output).toString(), 'abc')));

fn(deflate, () => {
fn(inflate, () => {
deflate.write('abc');
deflate.end();
});
});
}

0 comments on commit 252f376

Please sign in to comment.