Skip to content

Commit

Permalink
src: use single ObjectTemplate for TextDecoder
Browse files Browse the repository at this point in the history
`ObjectTemplate`s are not garbage-collected like regular objects
(for some reason). It is sufficient to create a single template
anyway, so do that to address the memory leak.

Fixes: #32424

PR-URL: #32426
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
addaleax authored and targos committed Apr 28, 2020
1 parent c126d28 commit 2da974e
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 4 deletions.
1 change: 1 addition & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,7 @@ constexpr size_t kFsStatsBufferLength =
V(http2settings_constructor_template, v8::ObjectTemplate) \
V(http2stream_constructor_template, v8::ObjectTemplate) \
V(http2ping_constructor_template, v8::ObjectTemplate) \
V(i18n_converter_template, v8::ObjectTemplate) \
V(libuv_stream_wrap_ctor_template, v8::FunctionTemplate) \
V(message_port_constructor_template, v8::FunctionTemplate) \
V(pipe_constructor_template, v8::FunctionTemplate) \
Expand Down
14 changes: 12 additions & 2 deletions src/node_i18n.cc
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ namespace node {

using v8::Context;
using v8::FunctionCallbackInfo;
using v8::FunctionTemplate;
using v8::HandleScope;
using v8::Int32;
using v8::Isolate;
Expand Down Expand Up @@ -171,8 +172,7 @@ class ConverterObject : public BaseObject, Converter {
Environment* env = Environment::GetCurrent(args);
HandleScope scope(env->isolate());

Local<ObjectTemplate> t = ObjectTemplate::New(env->isolate());
t->SetInternalFieldCount(ConverterObject::kInternalFieldCount);
Local<ObjectTemplate> t = env->i18n_converter_template();
Local<Object> obj;
if (!t->NewInstance(env->context()).ToLocal(&obj)) return;

Expand Down Expand Up @@ -821,6 +821,16 @@ void Initialize(Local<Object> target,
env->SetMethod(target, "transcode", Transcode);

// ConverterObject
{
Local<FunctionTemplate> t = FunctionTemplate::New(env->isolate());
t->InstanceTemplate()->SetInternalFieldCount(
ConverterObject::kInternalFieldCount);
Local<String> converter_string =
FIXED_ONE_BYTE_STRING(env->isolate(), "Converter");
t->SetClassName(converter_string);
env->set_i18n_converter_template(t->InstanceTemplate());
}

env->SetMethod(target, "getConverter", ConverterObject::Create);
env->SetMethod(target, "decode", ConverterObject::Decode);
env->SetMethod(target, "hasConverter", ConverterObject::Has);
Expand Down
9 changes: 7 additions & 2 deletions test/parallel/test-whatwg-encoding-custom-textdecoder.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,13 @@ if (common.hasIntl) {
if (common.hasIntl) {
assert.strictEqual(
util.inspect(dec, { showHidden: true }),
'TextDecoder {\n encoding: \'utf-8\',\n fatal: false,\n ' +
'ignoreBOM: true,\n [Symbol(flags)]: 4,\n [Symbol(handle)]: {}\n}'
'TextDecoder {\n' +
' encoding: \'utf-8\',\n' +
' fatal: false,\n' +
' ignoreBOM: true,\n' +
' [Symbol(flags)]: 4,\n' +
' [Symbol(handle)]: Converter {}\n' +
'}'
);
} else {
assert.strictEqual(
Expand Down

0 comments on commit 2da974e

Please sign in to comment.