Memory leak in vm.compileFunction when using importModuleDynamically

Version

17.5.0, 16.14.0, does not happen in 14.19.0

Platform

Microsoft Windows NT 10.0.22557.0 x64

Subsystem

vm

What steps will reproduce the bug?

// test.js
const vm = require('vm')

const code = `console.log("${'hello world '.repeat(1e5)}");`

while (true) {
  for (let i = 0; i < 30; i++)
    vm.compileFunction(code, [], {
      // comment out the following line to make it no longer leaking memory
      importModuleDynamically: () => {},
    })
  if (typeof gc !== 'undefined') gc()
  console.log(
    Math.round(process.memoryUsage().heapUsed / 1024 / 10.24) / 100,
    'MiB'
  )
}

Run this piece of code with node --expose-gc test.js.

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior?

Memory usage should stay constant.

You can also comment out the importModuleDynamically option to see the expected behavior.

Here is what I see:

4.32 MiB
4.34 MiB
4.33 MiB
4.33 MiB
4.33 MiB
4.33 MiB
4.34 MiB
4.34 MiB
4.34 MiB
4.34 MiB
4.35 MiB

What do you see instead?

You will see memory usage increasing in an unexpected way.

Here is what I see:

5.46 MiB
5.54 MiB
5.57 MiB
5.6 MiB
5.62 MiB
5.66 MiB
5.68 MiB
5.71 MiB
5.97 MiB
5.78 MiB
5.81 MiB
5.85 MiB
5.88 MiB
5.91 MiB
5.94 MiB
5.98 MiB
6 MiB
6.03 MiB
6.06 MiB
6.09 MiB
6.12 MiB
6.15 MiB
6.19 MiB
6.22 MiB
6.25 MiB
6.28 MiB
6.31 MiB
6.34 MiB
6.37 MiB
6.4 MiB
6.43 MiB

Additional information

There is more information in this issue: vercel/next.js#34659 (comment)

I think this leak was introduced by this commit: bf2f2b7#diff-c1d48dc599e8281b0be28ab449ea52917f6d4cc0578adb61ae99c631078b1a41R387

This commit could also be related: 89e4b36

Here is the code that causes the leak in my opinion:

node/lib/vm.js

Lines 380 to 382 in 45b5ca8

    
           callbackMap.set(result.cacheKey, { 
        
             importModuleDynamically: (s, _k, i) => wrapped(s, func, i), 
        
           });

Here is another piece of code that could be relevant:

node/src/node_contextify.cc

Line 1197 in 6847fec

CompiledFnEntry* entry = new CompiledFnEntry(env, cache_key, id, script);

And this piece of code:

node/src/node_contextify.cc

Lines 1234 to 1248 in 6847fec

    
           void CompiledFnEntry::WeakCallback( 
        
               const WeakCallbackInfo<CompiledFnEntry>& data) { 
        
             CompiledFnEntry* entry = data.GetParameter(); 
        
             delete entry; 
        
           } 
        
           CompiledFnEntry::CompiledFnEntry(Environment* env, 
        
                                            Local<Object> object, 
        
                                            uint32_t id, 
        
                                            Local<ScriptOrModule> script) 
        
               : BaseObject(env, object), 
        
                 id_(id), 
        
                 script_(env->isolate(), script) { 
        
             script_.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter); 
        
           }

And there is this documentation about BaseObject: https://github.com/nodejs/node/tree/master/src#lifetime-management

BaseObject also defines a MakeWeak, which is not used in that case, but might be potentially relevant:

node/src/base_object-inl.h

Lines 111 to 130 in 470c284

    
           void BaseObject::MakeWeak() { 
        
             if (has_pointer_data()) { 
        
               pointer_data()->wants_weak_jsobj = true; 
        
               if (pointer_data()->strong_ptr_count > 0) return; 
        
             } 
        
             persistent_handle_.SetWeak( 
        
                 this, 
        
                 [](const v8::WeakCallbackInfo<BaseObject>& data) { 
        
                   BaseObject* obj = data.GetParameter(); 
        
                   // Clear the persistent handle so that ~BaseObject() doesn't attempt 
        
                   // to mess with internal fields, since the JS object may have 
        
                   // transitioned into an invalid state. 
        
                   // Refs: https://github.com/nodejs/node/issues/18897 
        
                   obj->persistent_handle_.Reset(); 
        
                   CHECK_IMPLIES(obj->has_pointer_data(), 
        
                                 obj->pointer_data()->strong_ptr_count == 0); 
        
                   obj->OnGCCollect(); 
        
                 }, v8::WeakCallbackType::kParameter); 
        
           }

@nodejs/modules

Any progress on this one?

I wonder if this is what I am experiencing as well. A memory leak with thousands of CompiledFnEntry objects in the "containment" view of devtools.

Per this issue #25424 (comment) and this comment on the underlying V8 issue they blame --expose-gc as causing the problem. To make any progress on this I think the issue needs to be reproducible without the --expose-gc.

I think #44211 is a better repro case for that as it can be reproduced without it.

To make any progress on this I think the issue needs to be reproducible without the --expose-gc.

It is, --expose-gc is optional for the repro. You can also run it without (which will make number more unreliable, but the leak is still there).

node --max-heap-size=20 test.js
7.28 MiB
7.36 MiB
7.39 MiB
7.42 MiB
7.45 MiB
7.71 MiB
7.74 MiB
7.77 MiB
7.8 MiB
7.82 MiB
7.85 MiB
...
15.58 MiB
15.63 MiB
15.68 MiB
15.72 MiB
15.58 MiB
15.63 MiB
15.67 MiB
15.72 MiB
15.76 MiB
15.92 MiB
15.95 MiB
15.97 MiB

<--- Last few GCs --->
al[19616:000001CE737A4A50]    38145 ms: Mark-sweep (reduce) 15.7 (18.1) -> 15.5 (17.8) MB, 9.5 / 0.0 ms  (+ 0.2 ms in 3 steps since start of marking, biggest step 0.1 ms, walltime since start of marking 984 ms) (average mu = 0.996, current mu = 0.990) final[19616:000001CE737A4A50]    39115 ms: Mark-sweep (reduce) 16.0 (18.1) -> 15.7 (18.3) MB, 1.9 / 0.0 ms  (+ 0.2 ms in 3 steps since start of marking, biggest step 0.1 ms, walltime since start of marking 970 ms) (average mu = 0.997, current mu = 0.998) final

<--- JS stacktrace --->

FATAL ERROR: Reached heap limit Allocation failed - JavaScript heap out of memory
 1: 00007FF6A8571C0F node_api_throw_syntax_error+182911
 2: 00007FF6A84FF2F6 v8::internal::MicrotaskQueue::GetMicrotasksScopeDepth+67078
 3: 00007FF6A850043D node::OnFatalError+301
 4: 00007FF6A8F921CE v8::Isolate::ReportExternalAllocationLimitReached+94
 5: 00007FF6A8F7D552 v8::Isolate::Exit+674
 6: 00007FF6A8DFF5AC v8::internal::EmbedderStackStateScope::ExplicitScopeForTesting+124
 7: 00007FF6A8DFC7CB v8::internal::Heap::CollectGarbage+3963
 8: 00007FF6A8E04425 v8::internal::Heap::GlobalSizeOfObjects+341
 9: 00007FF6A8E53A9F v8::internal::StackGuard::HandleInterrupts+863
10: 00007FF6A8B1599F v8::internal::DateCache::Weekday+7327
11: 00007FF6A902F871 v8::internal::SetupIsolateDelegate::SetupHeap+558193
12: 00007FF6A8FB3E15 v8::internal::SetupIsolateDelegate::SetupHeap+51733
13: 00007FF629184654

Disable dynamic import for unix-dgram causing memory leak

I have a fix for vm.compileFunction() itself in #46785 - although the module constructors are still leaking, and that'd take a V8 CL to fix, but at least we could fix vm.compileFunction() first. The analysis is posted in #44211 (comment).

vm: fix leak in vm.compileFunction when importModuleDynamically is used

The fix ended up breaking many people and was reverted in #47101

I suspect that the compiled function is not enough to keep the CompiledFnEntry alive, which leads to the use-after-free in #47096, though from only looking at the code it appears that should be, but I'll need to do some debugging to find out if the referrer function is the same as the one we are storing references with.

Any updates on this post-revert? We are burned by this memory leak here constantly

module: use symbol in WeakMap to manage host defined options

module: fix leak of vm.SyntheticModule

module: fix the leak in SourceTextModule and ContextifySript

vm: fix leak in vm.compileFunction when importModuleDynamically is used

Navigation Menu

Repository navigation

Memory leak in `vm.compileFunction` when using `importModuleDynamically` #42080

Version

Platform

Subsystem

What steps will reproduce the bug?

How often does it reproduce? Is there a required condition?

What is the expected behavior?

What do you see instead?

Additional information

25 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	callbackMap.set(result.cacheKey, {
	importModuleDynamically: (s, _k, i) => wrapped(s, func, i),
	});

	void CompiledFnEntry::WeakCallback(
	const WeakCallbackInfo<CompiledFnEntry>& data) {
	CompiledFnEntry* entry = data.GetParameter();
	delete entry;
	}

	CompiledFnEntry::CompiledFnEntry(Environment* env,
	Local<Object> object,
	uint32_t id,
	Local<ScriptOrModule> script)
	: BaseObject(env, object),
	id_(id),
	script_(env->isolate(), script) {
	script_.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter);
	}

	void BaseObject::MakeWeak() {
	if (has_pointer_data()) {
	pointer_data()->wants_weak_jsobj = true;
	if (pointer_data()->strong_ptr_count > 0) return;
	}

	persistent_handle_.SetWeak(
	this,
	[](const v8::WeakCallbackInfo<BaseObject>& data) {
	BaseObject* obj = data.GetParameter();
	// Clear the persistent handle so that ~BaseObject() doesn't attempt
	// to mess with internal fields, since the JS object may have
	// transitioned into an invalid state.
	// Refs: https://github.com/nodejs/node/issues/18897
	obj->persistent_handle_.Reset();
	CHECK_IMPLIES(obj->has_pointer_data(),
	obj->pointer_data()->strong_ptr_count == 0);
	obj->OnGCCollect();
	}, v8::WeakCallbackType::kParameter);
	}

Memory leak in vm.compileFunction when using importModuleDynamically #42080

Description

Version

Platform

Subsystem

What steps will reproduce the bug?

How often does it reproduce? Is there a required condition?

What is the expected behavior?

What do you see instead?

Additional information

Activity

benjamingr commented on Feb 23, 2022

sokra commented on Jun 1, 2022

megahypercat commented on Jun 17, 2022

phawxby commented on Aug 18, 2022

sokra commented on Aug 23, 2022

joyeecheung commented on Feb 23, 2023

targos commented on Mar 15, 2023

joyeecheung commented on Mar 15, 2023

turtlesoupy commented on Mar 29, 2023

25 remaining items

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Memory leak in `vm.compileFunction` when using `importModuleDynamically` #42080