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

fix: Fix the memory leak #1119

Open
wants to merge 1 commit into
base: 0.13
Choose a base branch
from
Open

Conversation

theofidry
Copy link
Contributor

@theofidry theofidry commented May 2, 2023

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? could not check
Documented? no
Fixed tickets #721
License MIT

Follow up of my comment.

I cannot investigate further, but my guess is that exceptions are not cheap objects. When creating an exception you also capture the context and stack trace which means that even if the extension is unsued:

  • It slows down the program because instantiating it is not cheap.
  • May prevent some objects to be garbage collected because referenced in the exception.

Closes overblog#721.

Follow up of [my
comment](overblog#721 (comment)).

I cannot investigate further, but my guess is that exceptions are not
cheap objects. When creating an exception you also capture the context
and stack trace which means that even if the extension is unsued:

- it slows down the program because instantiating it is not cheap
- may prevent some objects to be garbage collected because referenced in
  the exception
@theofidry theofidry changed the base branch from master to 0.13 May 2, 2023 13:22
@Vincz
Copy link
Collaborator

Vincz commented May 3, 2023

Hi @theofidry!
Can you check the tests please?
Don't have time to dig into this atm but it seems to break the tests suite: https://github.com/overblog/GraphQLBundle/pull/1119/checks

Thanks!

@Vincz
Copy link
Collaborator

Vincz commented May 29, 2023

Hi @theofidry! Any update on this topic? Thanks!

@Vincz Vincz mentioned this pull request May 29, 2023
@theofidry
Copy link
Contributor Author

Sorry will try to come back to this in a near future

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

Successfully merging this pull request may close these issues.

2 participants