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

using class-validator in any file causes a memory leak #580

Closed
gevorggalstyan opened this issue Mar 15, 2020 · 8 comments
Closed

using class-validator in any file causes a memory leak #580

gevorggalstyan opened this issue Mar 15, 2020 · 8 comments
Labels
Community 👨‍👧 Something initiated by a community Invalid 👎 This doesn't seem right

Comments

@gevorggalstyan
Copy link

Describe the bug
importing anything from "class-validator" anywhere in the code causes memory leaks and eventually, the GC heaps crash. More specifically we are using typegraphql in a serverless stack and we have noticed that our local environment crashes with no reason with a GC heap overload. After some debugging and memory snapshots comparison we have figured out that whenever we use anything from the "class-validator" we have a memory leak. The memory consumption grows by a couple of MBs on each request and eventually, it crashes.

To Reproduce
Just bootstrap a minimal "apollo-server-lambda" serverless app, create a simple input type with a field validator and use apache benchmark to bombard the server. For a quicker result start the serverless with NODE_OPTIONS='--max-old-space-size=512', otherwise on 64bit systems it is 1400MB and it takes a while to crash it. You can even see how memory consumption grows while making requests. The code below is enough to reproduce the bug.

import { IsUUID } from "class-validator";
import { Field, ID, InputType } from "type-graphql";

@InputType()
export class MyInput {
  @Field(() => ID)
  @IsUUID()
  public id!: string;
}

and here is the simple command to accelerate the repro

ab -c 10 -n 2500 http://127.0.0.1:4567/local/v1/graphql/

Expected behavior
Memory should stay clear and have no leaks and the application should not crash.

Logs


<--- Last few GCs --->

[24532:0x10291f000]   242805 ms: Mark-sweep 503.7 (518.5) -> 499.5 (517.5) MB, 207.3 / 0.0 ms  (average mu = 0.135, current mu = 0.045) allocation failure scavenge might not succeed
[24532:0x10291f000]   243037 ms: Mark-sweep 506.3 (519.8) -> 503.5 (515.8) MB, 9.6 / 0.0 ms  (+ 119.5 ms in 39 steps since start of marking, biggest step 7.4 ms, walltime since start of marking 182 ms) (average mu = 0.283, current mu = 0.445) finalize inc

<--- JS stacktrace --->

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

    0: ExitFrame [pc: 0x100950919]
Security context: 0x12ab9f8408d1 <JSObject>
    1: readToken(aka readToken) [0x12ab3657bbd1] [/Users/g/Sites/m/node_modules/graphql/language/lexer.js:~112] [pc=0x10bf298d85f0](this=0x12ab8e5404b1 <undefined>,0x12ab71daed39 <Object map = 0x12ab5e703a49>,0x12ab96280591 <Tok map = 0x12ab18cd3fd9>)
    2: advanceLexer [0x12ab3657bc11] [/Users/g/Sites/m/consumer-notific...

FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory

Writing Node.js report to file: report.20200316.021428.24532.0.001.json
Node.js report completed
 1: 0x100080c68 node::Abort() [/Users/g/.nvm/versions/node/v12.16.1/bin/node]
 2: 0x100080dec node::errors::TryCatchScope::~TryCatchScope() [/Users/g/.nvm/versions/node/v12.16.1/bin/node]
 3: 0x100185167 v8::Utils::ReportOOMFailure(v8::internal::Isolate*, char const*, bool) [/Users/g/.nvm/versions/node/v12.16.1/bin/node]
 4: 0x100185103 v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate*, char const*, bool) [/Users/g/.nvm/versions/node/v12.16.1/bin/node]
 5: 0x10030b2f5 v8::internal::Heap::FatalProcessOutOfMemory(char const*) [/Users/g/.nvm/versions/node/v12.16.1/bin/node]
 6: 0x10030c9c4 v8::internal::Heap::RecomputeLimits(v8::internal::GarbageCollector) [/Users/g/.nvm/versions/node/v12.16.1/bin/node]
 7: 0x100309837 v8::internal::Heap::PerformGarbageCollection(v8::internal::GarbageCollector, v8::GCCallbackFlags) [/Users/g/.nvm/versions/node/v12.16.1/bin/node]
 8: 0x1003077fd v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollectionReason, v8::GCCallbackFlags) [/Users/g/.nvm/versions/node/v12.16.1/bin/node]
 9: 0x100312fba v8::internal::Heap::AllocateRawWithLightRetry(int, v8::internal::AllocationType, v8::internal::AllocationOrigin, v8::internal::AllocationAlignment) [/Users/g/.nvm/versions/node/v12.16.1/bin/node]
10: 0x100313041 v8::internal::Heap::AllocateRawWithRetryOrFail(int, v8::internal::AllocationType, v8::internal::AllocationOrigin, v8::internal::AllocationAlignment) [/Users/g/.nvm/versions/node/v12.16.1/bin/node]
11: 0x1002e035b v8::internal::Factory::NewFillerObject(int, bool, v8::internal::AllocationType, v8::internal::AllocationOrigin) [/Users/g/.nvm/versions/node/v12.16.1/bin/node]
12: 0x100618718 v8::internal::Runtime_AllocateInYoungGeneration(int, unsigned long*, v8::internal::Isolate*) [/Users/g/.nvm/versions/node/v12.16.1/bin/node]
13: 0x100950919 Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_NoBuiltinExit [/Users/g/.nvm/versions/node/v12.16.1/bin/node]
14: 0x10bf298d85f0
15: 0x10bf29a3c6f4
[1]    24531 abort      npm start

Enviorment (please complete the following information):

  • OS: macOS Catalina 10.15.3
  • Node 12.16.1
  • Package version 0.17.6
  • TypeScript version 3.8.3

Additional context
This behavior has been observed on node versions from 10-12. we did not test on other versions.

@MichalLytek MichalLytek added Community 👨‍👧 Something initiated by a community Need More Info 🤷‍♂️ Further information is requested labels Mar 16, 2020
@MichalLytek
Copy link
Owner

I believe that the root of the issue is the same for both TypeGraphQL and class-validator - they store the class metadata in a global array.

And it works fine, until you try to run it in an environment like lambda and when you try to build the schema on every request.

As stated in many issues in this repo, you should build the schema only once and put into global. scope and then retrive it from there - it will save a tons of money from not wasting time on building the schema again and again, as well as prevent memory leaks that are caused by re-requiring all the classes and building the schema again.

@gevorggalstyan
Copy link
Author

@MichalLytek Thanks for getting back. A few observations related to that.

  1. We are using webpack BannerPlugin as such to get rid of some metadata
new BannerPlugin({
      banner: `
        require("reflect-metadata");
        delete global.TypeGraphQLMetadataStorage;
        delete global.typeormMetadataArgsStorage;
      `,
      entryOnly: true,
      raw: true,
    }),

We have added this to get rid of duplicates warning on subsequent requests. And yes you are right, we see the globals getting more and more of the compiled code strings, but that does not get stuck in the memory and GC is able to clean that up.

  1. The leak is not only happening on the local env, but actually on the real deployed apps. And the result is very very hard to catch. Sometimes the aws lambda functions get invoked but never instantiated because of how aws lambda freezes and unfreezes containers (including GC heap).

  2. Taking into account all of the above, do you think deleting something from global may help as we do already? If yes, what would that be?

@MichalLytek
Copy link
Owner

Based on the code:
https://github.com/typestack/class-validator/blob/master/src/metadata/MetadataStorage.ts

Every decorator call (another require) results in pushing to the big array, so this results in memory leaks as it store the metadata for classes that are not used anymore.

Maybe you need to raise and issue on class-validator repo as well.

@MichalLytek
Copy link
Owner

We have added this to get rid of duplicates warning on subsequent requests.

I think you should change the way how your code is called, otherwise it's just dirty patching, not fixing the root cause.

@gevorggalstyan
Copy link
Author

I think you should change the way how your code is called, otherwise it's just dirty patching, not fixing the root cause.

Unfortunately, I don't think there is a way to have this working on the local correctly. Note that we don't need to do that on real deployed apps.

@MichalLytek
Copy link
Owner

MichalLytek commented Mar 16, 2020

The leak is not only happening on the local env, but actually on the real deployed apps.

Note that we don't need to do that on real deployed apps.

Please decide 😄

So the leak is happening on real apps too but does not crash the machine as on local env?

@gevorggalstyan
Copy link
Author

Haven’t you noticed the part about aws lambdas?

@MichalLytek
Copy link
Owner

@gevorggalstyan
Haven’t you noticed the part about other lambda issues?
#96
#162
#443

You're report is unclear and you're not helping with not answering to questions.

Closing this issue as it looks like a class-validator metadata storage leak 🔒
Please open an issue on theirs repository.

TypeGraphQL metadata storage will be redesigned to use WeakMap (along with #183) which prevents memory leaks. This gonna take place in the vNext release that is scheduled somewhere in the future.

@MichalLytek MichalLytek added Invalid 👎 This doesn't seem right and removed Need More Info 🤷‍♂️ Further information is requested labels Mar 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community 👨‍👧 Something initiated by a community Invalid 👎 This doesn't seem right
Projects
None yet
Development

No branches or pull requests

2 participants