Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory not cleaned up after worker threads are terminated #45685

Closed
notakay opened this issue Nov 30, 2022 · 5 comments
Closed

Memory not cleaned up after worker threads are terminated #45685

notakay opened this issue Nov 30, 2022 · 5 comments

Comments

@notakay
Copy link

notakay commented Nov 30, 2022

Version

v16.14.0

Platform

Darwin akays.local 22.1.0 Darwin Kernel Version 22.1.0: Sun Oct 9 20:14:30 PDT 2022; root:xnu-8792.41.9~2/RELEASE_ARM64_T8103 arm64

Subsystem

worker_threads

What steps will reproduce the bug?

const { memoryUsage } = require('node:process')
const { Worker, isMainThread } = require('node:worker_threads')

if (isMainThread) {
    let counter = 0
    console.log(++counter, JSON.stringify(memoryUsage()))

    setInterval(() => {
        console.log(++counter, JSON.stringify(memoryUsage()))
    }, 5000)

    setInterval(() => {
        const worker = new Worker(__filename)
        worker.on('online', () => {
            worker.terminate()
        })
    }, 100)
} else {
    // some operation
    const x = 3
}

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

Always reproducible. Also tested on node v19.0.0

What is the expected behavior?

Memory usage should not grow as much.

What do you see instead?

Memory usage continues to grow.

1 {"rss":28246016,"heapTotal":4947968,"heapUsed":4102208,"external":305804,"arrayBuffers":11162}
2 {"rss":38584320,"heapTotal":5734400,"heapUsed":4837192,"external":305844,"arrayBuffers":11162}
3 {"rss":41320448,"heapTotal":6258688,"heapUsed":4969176,"external":305844,"arrayBuffers":11162}
4 {"rss":41697280,"heapTotal":5210112,"heapUsed":4769080,"external":303141,"arrayBuffers":10426}
5 {"rss":42188800,"heapTotal":5996544,"heapUsed":5071352,"external":303141,"arrayBuffers":10426}
6 {"rss":42483712,"heapTotal":6258688,"heapUsed":5265592,"external":303141,"arrayBuffers":10426}
7 {"rss":44433408,"heapTotal":6799360,"heapUsed":5616992,"external":303141,"arrayBuffers":10426}
8 {"rss":44548096,"heapTotal":7061504,"heapUsed":5705592,"external":303141,"arrayBuffers":10426}
9 {"rss":45219840,"heapTotal":7585792,"heapUsed":6312392,"external":303141,"arrayBuffers":10426}
10 {"rss":46104576,"heapTotal":7585792,"heapUsed":7002760,"external":303141,"arrayBuffers":10426}
11 {"rss":46317568,"heapTotal":8110080,"heapUsed":6972080,"external":303141,"arrayBuffers":10426}

Additional information

Initial memory usage

image

Memory usage after ~5 minutes

image

@bnoordhuis
Copy link
Member

Memory usage should not grow as much.

Define "as much"? I don't see anything out of the ordinary.

Are you getting confused by the main thread's GC growing the JS heap from time to time?

@ywave620
Copy link
Contributor

It seems that memory occuiped by Code is to blame for the memory growth. Maybe due to the memory leak in vm module, which is used by worker module to run script. See #44211

@bnoordhuis
Copy link
Member

worker_threads doesn't use vm but I see what you mean. 289 -> 705 kB over the course of five minutes isn't that worrisome, that's probably the JIT's optimizing compiler kicking in.

@notakay
Copy link
Author

notakay commented Nov 30, 2022

I have a more complex long running application with multiple workers.

The application logic is as follows:

  • The workers would report the memory usage at certain intervals.
  • If the memory usage is above a certain threshold (eg. 300MB), the main thread will terminate the worker and create a new one.
  • However, when the worker was terminated, only ~100MB was recovered.
    • A new thread takes ~80MB, so I expect to recover ~200MB.

The memory has grown from ~400MB to ~1GB over two weeks.

I don't know the cause for it yet, but from the POC I suspect it might might be from memory occupied by Code.

@notakay
Copy link
Author

notakay commented Nov 30, 2022

Closing this issue, since the bug was elsewhere, with the reference to a terminated worker not being cleaned up.
The code with the memory leak looks like the following:

// memory leaking version
const { memoryUsage } = require('node:process')
const { Worker, isMainThread } = require('node:worker_threads')

if (isMainThread) {
    const workers = []
    
    let counter = 0
    console.log(++counter, JSON.stringify(memoryUsage()))

    setInterval(() => {
        console.log(++counter, JSON.stringify(memoryUsage()))
    }, 5000)

    setInterval(() => {
        const worker = new Worker(__filename)
        worker.on('online', () => {
            worker.terminate()
        })
        
        workers.push(worker) // keeps reference to terminated worker worker
    }, 100)
} else {
    // some operation
    const x = 3
}

When running the poc code in the original pos, the heapTotal eventually maxed out at 12615680 bytes and did not grow anymore.

On the other hand, when running the buggier version, the heapTotal exceeded 12615680 bytes quickly and continued to grow.

Thank you both @bnoordhuis @ywave620

@notakay notakay closed this as completed Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants