Skip to content

Fix memory leak in exporter and reader#4224

Merged
lzchen merged 11 commits into
open-telemetry:mainfrom
msoltysik:fix-memory-leak
Oct 17, 2024
Merged

Fix memory leak in exporter and reader#4224
lzchen merged 11 commits into
open-telemetry:mainfrom
msoltysik:fix-memory-leak

Conversation

@msoltysik

@msoltysik msoltysik commented Oct 15, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

This PR addresses memory leak issue in the exporter and reader by using the weakref library.

The solution, proposed by @aabmass, utilizes weakref.WeakMethod and weakref.WeakSet to ensure proper garbage collection and prevent unintended object retention.

Discussion can be found in the following issue - Fixes #4220

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

It has been covered by a test case.

============================= test session starts ==============================
collecting ... collected 1 item

test_provider_shutdown.py::TestMeterProviderShutdown::test_meter_provider_shutdown_cleans_up_successfully PASSED [100%]

============================== 1 passed in 0.08s ===============================

Does This PR Require a Contrib Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@msoltysik msoltysik requested a review from a team as a code owner October 15, 2024 16:55
@linux-foundation-easycla

linux-foundation-easycla Bot commented Oct 15, 2024 •
edited
Loading

Copy link
Copy Markdown

CLA Signed

The committers listed above are authorized under a signed CLA.

@xrmx

xrmx commented Oct 16, 2024

Copy link
Copy Markdown
Contributor

@msoltysik please fix ruff failure

@msoltysik

msoltysik commented Oct 16, 2024

Copy link
Copy Markdown
Contributor Author

Hey @xrmx , I've just applied tox -e ruff

emdneto
emdneto approved these changes Oct 16, 2024
View reviewed changes

@emdneto emdneto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. The test fails against main. LGTM

@lzchen lzchen merged commit 5de1ccb into open-telemetry:main Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xrmx xrmx xrmx approved these changes
@emdneto emdneto emdneto approved these changes

Assignees

No one assigned

Labels

None yet

Projects

Python PR digest
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Unable to release memory

4 participants

@msoltysik @xrmx @emdneto @lzchen