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.

Sign up for GitHub

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

Jump to bottom

Possible memory leak in Object API #6193

Closed
2 of 7 tasks
akhenry opened this issue Jan 25, 2023 · 2 comments · Fixed by #6269
Closed
2 of 7 tasks

Possible memory leak in Object API #6193

akhenry opened this issue Jan 25, 2023 · 2 comments · Fixed by #6269
Assignees
@jvigliotta
Labels
severity:critical type:bug
Milestone
Target:2.1.6

Comments

@akhenry
Copy link
Contributor

akhenry commented Jan 25, 2023 •
edited
Loading

Summary

The object API will temporarily create a mutable in order to refresh an object's model. This is to make sure all holders of that object get a fresh model and we are not maintaining multiple different versions of a model in memory.

Whenever a mutable is created however, it must be destroyed by the code that created it. It does not appear that this is happening currently, so this is a potential source of memory leaks that should be fixed.

We should also investigate why we have logic checking whether an object returned by a provider is a mutable. Objects returned by .get() should never be mutable, this is why we have a distinction between .get() and .getMutable()

  • Open MCT Version: 2.1.6

Impact Check List

  • Data loss or misrepresented data?
  • Regression? Did this used to work or has it always been broken?
  • Is there a workaround available?
  • Does this impact a critical component?
  • Is this just a visual bug with no functional impact?
  • Does this block the execution of e2e tests?
  • Does this have an impact on Performance?
@akhenry
Copy link
Contributor Author

akhenry commented Jan 25, 2023

Filing is critical because of the impact of memory leaks on performance.

@akhenry akhenry added this to the Target:2.1.6 milestone Jan 25, 2023
@ozyx
Copy link
Contributor

ozyx commented Jan 25, 2023

Code was originally introduced here

@jvigliotta jvigliotta mentioned this issue Feb 2, 2023
Merged
15 tasks
akhenry pushed a commit that referenced this issue Feb 6, 2023
@jvigliotta @akhenry
cherry-pick(#6269): Possible memory leak in Object API (#6193)
6d6bacb 
* Destroy mutable after refresh
* Check if domainObject supports mutation before getting mutable
@akhenry akhenry mentioned this issue Feb 6, 2023
Merged
ozyx pushed a commit that referenced this issue Feb 6, 2023
@akhenry @jvigliotta
cherry-pick(#6269): [ObjectAPI] Cleanup code and remove possible memo…
c2957ac 
…ry leak (#6286)

cherry-pick(#6269): Possible memory leak in Object API (#6193)

* Destroy mutable after refresh
* Check if domainObject supports mutation before getting mutable

Co-authored-by: Jamie V <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jvigliotta jvigliotta

Labels
severity:critical type:bug
Projects
None yet
Milestone
Target:2.1.6
Development

Successfully merging a pull request may close this issue.

[ObjectAPI] Cleanup code and remove possible memory leak nasa/openmct
3 participants
@akhenry @ozyx @jvigliotta