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

Test.createTestingModule leaves open handle due to xxh32 #11072

Closed
1 of 2 tasks
urugator opened this issue Feb 7, 2023 · 5 comments
Closed
1 of 2 tasks

Test.createTestingModule leaves open handle due to xxh32 #11072

urugator opened this issue Feb 7, 2023 · 5 comments
Labels
needs triage This issue has not been looked into type: bug 😭

Comments

@urugator
Copy link

urugator commented Feb 7, 2023

Did you read the migration guide?

  • I have read the whole migration guide

Is there an existing issue that is already proposing this?

  • I have searched the existing issues

Potential Commit/PR that introduced the regression

d323edb

NestJS version

9.0.0 -> 9.3.5

Describe the regression

After upgrading from 9.0.0 to 9.3.5, tests are randomly reporting: "Jest did not exit one second after the test run has completed."
After running with --detectOpenHandles

Jest has detected the following 1 open handle potentially keeping Jest from exiting:

  ●  CustomGC

    > 1 | import { Test } from '@nestjs/testing';
        | ^
      2 |
      3 | Test.createTestingModule({});
      4 |

      at Runtime._loadModule (../../node_modules/jest-runtime/build/index.js:1218:29)
      at Object.<anonymous> (../../node_modules/@node-rs/xxhash/index.js:64:29)
      at Object.<anonymous> (../../node_modules/@nestjs/core/inspector/deterministic-uuid-registry.js:4:18)
      at Object.<anonymous> (../../node_modules/@nestjs/core/injector/instance-wrapper.js:11:24)
      at Object.<anonymous> (../../node_modules/@nestjs/core/injector/injector.js:14:28)
      at Object.<anonymous> (../../node_modules/@nestjs/core/injector/module-ref.js:8:20)
      at Object.<anonymous> (../../node_modules/@nestjs/core/injector/lazy-module-loader/lazy-module-loader.js:5:22)
      at Object.<anonymous> (../../node_modules/@nestjs/core/inspector/serialized-graph.js:8:30)
      at Object.<anonymous> (../../node_modules/@nestjs/core/injector/container.js:6:28)
      at Object.<anonymous> (../../node_modules/@nestjs/testing/testing-module.builder.js:6:21)
      at Object.<anonymous> (../../node_modules/@nestjs/testing/test.js:5:34)
      at Object.<anonymous> (../../node_modules/@nestjs/testing/index.js:11:22)
      at Object.<anonymous> (test/foo.spec.ts:1:1)

Investigating further, noticed this recent change
d323edb
So I tried to run xxh32 directly:

import { xxh32 } from '@node-rs/xxhash';
xxh32('foobarbaz');

producing similar output

Jest has detected the following 1 open handle potentially keeping Jest from exiting:


    > 1 | import { xxh32 } from '@node-rs/xxhash';
        | ^
      2 | xxh32('foobarbaz');
      3 | //import { Test } from '@nestjs/testing';
      4 |

      at Runtime._loadModule (../../node_modules/jest-runtime/build/index.js:1218:29)
      at Object.<anonymous> (../../node_modules/@node-rs/xxhash/index.js:64:29)
      at Object.<anonymous> (test/foo.spec.ts:1:1)

So I assume it's due to the uuid -> xxhash change.

Minimum reproduction code

No response

Input code

No response

Expected behavior

Jest exits properly.

Other

No response

@urugator urugator added needs triage This issue has not been looked into type: bug 😭 labels Feb 7, 2023
@micalevisk
Copy link
Member

duplicate of #11071

@micalevisk micalevisk closed this as not planned Won't fix, can't repro, duplicate, stale Feb 7, 2023
@urugator
Copy link
Author

urugator commented Feb 7, 2023

Hi, thank you for the prompt response. I would just like to point out that while this issue is caused by the same change, it's not related to the deployment process as #11071 and can't be fixed as such. So introducing the package in nestjs 10 as suggested is potentially still a problem.

@H4ad
Copy link
Contributor

H4ad commented Feb 7, 2023

Hey @urugator, thanks for creating an issue, we are working to remove the dependency in this version.

Also, I think you could create the same issue at https://github.com/napi-rs/node-rs/issues, I don't think they know that the package has a potential memory leak.

@kamilmysliwiec
Copy link
Member

@urugator would you like to create an issue here too https://github.com/napi-rs/node-rs/issues?

@urugator
Copy link
Author

urugator commented Feb 7, 2023

Done napi-rs/node-rs#658

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage This issue has not been looked into type: bug 😭
Projects
None yet
Development

No branches or pull requests

4 participants