Potential memory leak in fs::WriteString

Version: master
Subsystem: fs

I have not verified it yet, just discovered this when I went through the comments in node_file.cc to see if there is anything to update.

node/src/node_file.cc

Lines 1389 to 1391 in 040dd24

    
           buf = new char[len]; 
        
           // SYNC_CALL returns on error.  Make sure to always free the memory. 
        
           if (!is_async) delete_on_return.reset(buf);

Looks like in the async case, if the buffer was copied instead of being moved (in the external string cases up there), then the buf will not get deleted after the request is done. This seems to be introduced when the FSReqWrap:: Ownership was removed in 4b9ba9b
, and ReleaseEarly was no longer called upon destruction of FSReqWrap.

I am trying to come up with a fix, opening an issue in case this gets lost.

@jasnell

cc @jasnell @addaleax

==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: ???

Output from valgrind running test/parallel/test-fs-write.js

fs: fix memory leak in WriteString

Navigation Menu

Repository navigation

Potential memory leak in fs::WriteString #19356

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	buf = new char[len];
	// SYNC_CALL returns on error. Make sure to always free the memory.
	if (!is_async) delete_on_return.reset(buf);

Potential memory leak in fs::WriteString #19356

Description

Activity

joyeecheung commented on Mar 14, 2018

joyeecheung commented on Mar 14, 2018

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions