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

Generator functions - memory leak #30753

Closed
assaf-xm opened this issue Dec 1, 2019 · 6 comments
Closed

Generator functions - memory leak #30753

assaf-xm opened this issue Dec 1, 2019 · 6 comments
Labels
v8 engine Issues and PRs related to the V8 dependency.

Comments

@assaf-xm
Copy link

assaf-xm commented Dec 1, 2019

Environment:

  • Version: v12.13.1
  • Platform: Linux server 4.4.0-36-generic (Ubuntu - 20GB RAM)

Probably a V8 issue, but can severely affect the stability of node.js processes.
This issue doesn't reproduce with node 10.13, only with node 12.

With the below simple code, I manage to get an internal array with more than 112M cells which cause the 'invalid array length' fatal error.

This isn't a real 'out of memory' issue, but an array size limit error.
It reproduces also when increasing the old space size using '--max-old-space-size=8192' (process halts ~1.2GB in any case).

"use strict";
const co = require('co');

function* test() {
    for (let i = 0; i < 1000000000; i++) {
        function* a() {}
        yield* a(); 
        if (i % 1000000 == 0) {
            console.log("Cycle", i, process.memoryUsage().heapUsed);
        }
    }
}

co(function*(){
    yield* test();
})

After few minutes at cycle 105M, the node process crash with the following error:

Cycle 105000000 1176098488

<--- Last few GCs --->

[6422:0x3a3feb0]   262277 ms: Scavenge 1161.3 (1178.4) -> 1150.5 (1181.7) MB, 6.2 / 0.0 ms  (average mu = 0.939, current mu = 0.939) allocation failure 
[6422:0x3a3feb0]   262336 ms: Scavenge 1174.1 (1191.4) -> 1163.3 (1194.7) MB, 6.5 / 0.0 ms  (average mu = 0.939, current mu = 0.939) allocation failure 
[6422:0x3a3feb0]   262395 ms: Scavenge 1186.8 (1203.9) -> 1176.0 (1207.2) MB, 7.0 / 0.0 ms  (average mu = 0.939, current mu = 0.939) allocation failure 


<--- JS stacktrace --->

==== JS stack trace =========================================

    0: ExitFrame [pc: 0x1374fd9]
Security context: 0x2d89a7ac08a1 <JSObject>
    1: test(aka test) [0x3038f06001e9] [/home/test_generators/test_generators.js:~4] [pc=0x3dbc7f842f1e](this=0x38a3372404a9 <undefined>)
    2: next [0x2d89a7ae3651](this=0x3038f0600149 <JSGenerator>,0x38a3372404a9 <undefined>)
    3: /* anonymous */ [0x3038f06002c9] [/home/test_generators/test_generators.js:15] [bytecode=0x10099225ec79 offset=88](th...

FATAL ERROR: invalid array length Allocation failed - JavaScript heap out of memory
 1: 0x9da7c0 node::Abort() [node]
 2: 0x9db976 node::OnFatalError(char const*, char const*) [node]
 3: 0xb39f1e v8::Utils::ReportOOMFailure(v8::internal::Isolate*, char const*, bool) [node]
 4: 0xb3a299 v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate*, char const*, bool) [node]
 5: 0xce5635  [node]
 6: 0xcc24a5 v8::internal::Factory::CopyWeakArrayListAndGrow(v8::internal::Handle<v8::internal::WeakArrayList>, int, v8::internal::AllocationType) [node]
 7: 0xebe86a v8::internal::WeakArrayList::EnsureSpace(v8::internal::Isolate*, v8::internal::Handle<v8::internal::WeakArrayList>, int, v8::internal::AllocationType) [node]
 8: 0xebeb3b v8::internal::PrototypeUsers::Add(v8::internal::Isolate*, v8::internal::Handle<v8::internal::WeakArrayList>, v8::internal::Handle<v8::internal::Map>, int*) [node]
 9: 0xe88054 v8::internal::JSObject::LazyRegisterPrototypeUser(v8::internal::Handle<v8::internal::Map>, v8::internal::Isolate*) [node]
10: 0xeb1384 v8::internal::Map::GetOrCreatePrototypeChainValidityCell(v8::internal::Handle<v8::internal::Map>, v8::internal::Isolate*) [node]
11: 0xd68e5c v8::internal::LoadHandler::LoadFromPrototype(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Map>, v8::internal::Handle<v8::internal::JSReceiver>, v8::internal::Handle<v8::internal::Smi>, v8::internal::MaybeObjectHandle, v8::internal::MaybeObjectHandle) [node]
12: 0xd70e67 v8::internal::LoadIC::ComputeHandler(v8::internal::LookupIterator*) [node]
13: 0xd77bcd v8::internal::LoadIC::UpdateCaches(v8::internal::LookupIterator*) [node]
14: 0xd7824c v8::internal::LoadIC::Load(v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Name>) [node]
15: 0xd7cf01 v8::internal::Runtime_LoadIC_Miss(int, unsigned long*, v8::internal::Isolate*) [node]
16: 0x1374fd9  [node]
Aborted (core dumped)

Should I open a bug to V8 regarding this issue?

@devsnek devsnek added the v8 engine Issues and PRs related to the V8 dependency. label Dec 3, 2019
@devsnek
Copy link
Member

devsnek commented Dec 3, 2019

You should report it to V8 (https://crbug.com/v8). If possible, you should also try to get it reproducing without co.

@assaf-xm
Copy link
Author

assaf-xm commented Dec 3, 2019

OK, opened an issue for V8: https://bugs.chromium.org/p/v8/issues/detail?id=10031

@devsnek devsnek closed this as completed Dec 3, 2019
@assaf-xm
Copy link
Author

@devsnek note that the issue was fixed and merged to V8 master: v8/v8@d3a1a5b

When/How it will get into node12?

@targos
Copy link
Member

targos commented Dec 17, 2019

Thanks, I'll reopen the issue since there's a fix to backport.

@targos targos reopened this Dec 17, 2019
targos added a commit to targos/node that referenced this issue Dec 17, 2019
Original commit message:

    [objects] Fix memory leak in PrototypeUsers::Add

    PrototypeUsers::Add now iterates the WeakArrayList to find empty slots
    before growing the array. Not reusing empty slots caused a memory leak.

    It might also be desirable to shrink the WeakArrayList in the future.
    Right now it is only compacted when invoking CreateBlob.

    Also removed unused PrototypeUsers::IsEmptySlot declaration.

    Bug: v8:10031
    Change-Id: I570ec78fca37e8f0c794f1f40846a4daab47c225
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1967317
    Reviewed-by: Ulan Degenbaev <[email protected]>
    Reviewed-by: Igor Sheludko <[email protected]>
    Commit-Queue: Dominik Inführ <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#65456}

Refs: v8/v8@d3a1a5b
Fixes: nodejs#30753
@targos
Copy link
Member

targos commented Dec 17, 2019

#31005

@targos targos closed this as completed in 05041d3 Dec 23, 2019
@alex3d
Copy link

alex3d commented Dec 24, 2019

@targos Will it get into node12?

BridgeAR pushed a commit that referenced this issue Jan 3, 2020
Original commit message:

    [objects] Fix memory leak in PrototypeUsers::Add

    PrototypeUsers::Add now iterates the WeakArrayList to find empty slots
    before growing the array. Not reusing empty slots caused a memory leak.

    It might also be desirable to shrink the WeakArrayList in the future.
    Right now it is only compacted when invoking CreateBlob.

    Also removed unused PrototypeUsers::IsEmptySlot declaration.

    Bug: v8:10031
    Change-Id: I570ec78fca37e8f0c794f1f40846a4daab47c225
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1967317
    Reviewed-by: Ulan Degenbaev <[email protected]>
    Reviewed-by: Igor Sheludko <[email protected]>
    Commit-Queue: Dominik Inführ <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#65456}

Refs: v8/v8@d3a1a5b
Fixes: #30753

PR-URL: #31005
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
targos added a commit that referenced this issue Jan 14, 2020
Original commit message:

    [objects] Fix memory leak in PrototypeUsers::Add

    PrototypeUsers::Add now iterates the WeakArrayList to find empty slots
    before growing the array. Not reusing empty slots caused a memory leak.

    It might also be desirable to shrink the WeakArrayList in the future.
    Right now it is only compacted when invoking CreateBlob.

    Also removed unused PrototypeUsers::IsEmptySlot declaration.

    Bug: v8:10031
    Change-Id: I570ec78fca37e8f0c794f1f40846a4daab47c225
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1967317
    Reviewed-by: Ulan Degenbaev <[email protected]>
    Reviewed-by: Igor Sheludko <[email protected]>
    Commit-Queue: Dominik Inführ <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#65456}

Refs: v8/v8@d3a1a5b
Fixes: #30753

PR-URL: #31005
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
BethGriggs pushed a commit that referenced this issue Feb 6, 2020
Original commit message:

    [objects] Fix memory leak in PrototypeUsers::Add

    PrototypeUsers::Add now iterates the WeakArrayList to find empty slots
    before growing the array. Not reusing empty slots caused a memory leak.

    It might also be desirable to shrink the WeakArrayList in the future.
    Right now it is only compacted when invoking CreateBlob.

    Also removed unused PrototypeUsers::IsEmptySlot declaration.

    Bug: v8:10031
    Change-Id: I570ec78fca37e8f0c794f1f40846a4daab47c225
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1967317
    Reviewed-by: Ulan Degenbaev <[email protected]>
    Reviewed-by: Igor Sheludko <[email protected]>
    Commit-Queue: Dominik Inführ <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#65456}

Refs: v8/v8@d3a1a5b
Fixes: #30753

PR-URL: #31005
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants