Skip to content

fs: fix memory leak in WriteString

Rodrigo Muino Tomonari requested to merge github/fork/joyeecheung/fix-writestring-leak into master

In the async case, if the buffer was copied instead of being moved then the buf will not get deleted after the request is done. This was introduced when the FSReqWrap:: Ownership was removed in 4b9ba9b8, and ReleaseEarly was no longer called upon destruction of FSReqWrap.

Create a custom Init function so we can use the MaybeStackBuffer in the FSReqBase to copy the string in the async case. The data will then get destructed when the FSReqBase is destructed.

Fixes: https://github.com/nodejs/node/issues/19356

Before this patch, running valgrind --leak-check=yes ./node --expose_externalize_string test/parallel/test-fs-write.js produces:

==25808== 21 bytes in 3 blocks are definitely lost in loss record 7 of 51
==25808==    at 0x4C2E80F: operator new[](unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==25808==    by 0x8238CA: node::fs::WriteString(v8::FunctionCallbackInfo<v8::Value> const&) (in /home/ubuntu/projects/node/out/Release/node)
==25808==    by 0x97873B: v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&)) (in /home/ubuntu/projects/node/out/Release/node)
==25808==    by 0x9CFEA4: v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) (in /home/ubuntu/projects/node/out/Release/node)
==25808==    by 0x9CF5D5: v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments, v8::internal::Isolate*) (in /home/ubuntu/projects/node/out/Release/node)
==25808==    by 0x21191EE0427C: ???
==25808==    by 0x21191EE13616: ???
==25808==    by 0x21191EE0BF02: ???
==25808==    by 0x21191EE13616: ???
==25808==    by 0x21191EE13616: ???
==25808==    by 0x21191EE0BF02: ???
==25808==    by 0x21191EE13616: ???

After this patch, this leak is gone.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

Merge request reports