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

Investigate and Fix Potential Memory Leak in at_thread_exit Machinery Using CRT Block Technique #4986

Closed
wants to merge 1 commit into from

Conversation

Arup-Chauhan
Copy link
Contributor

@Arup-Chauhan Arup-Chauhan commented Sep 27, 2024

Raised this draft PR investigates a potential memory leak in the at_thread_exit machinery using _CrtDumpMemoryLeaks (#4418)

If the leak is confirmed will fix it using the CRT block technique.

Currently, I will follow these steps:

  1. Investigate Memory Leak: Use _CrtDumpMemoryLeaks to check if the leak occurs during at_thread_exit.
  2. Apply CRT Block Technique: If a leak is found, wrap the problematic code with memory tracking blocks to manage allocations properly.
  3. Verify the Fix: Run the test again to ensure no memory leaks are reported after applying the fix.
  4. Add a Unit Test: Create a test case that validates the absence of memory leaks in the at_thread_exit machinery.

Feedback is welcome on the initial investigation and direction.

Signed-off-by: Arup Chauhan <arupchauhan26@gmail.com>
@frederick-vs-ja
Copy link
Contributor

frederick-vs-ja commented Sep 27, 2024

Not sure why a PR is opened at the current stage. Perhaps we can try to add unit cast first to catch the bug and then fix it.

@Arup-Chauhan
Copy link
Contributor Author

Arup-Chauhan commented Sep 27, 2024

Not sure why a PR is opened at the current stage. Perhaps we can try to add unit cast first to catch the bug and then fix it.

I raised the PR to document my steps, hence kept the PR in the draft stage, will keep updating this as I work on this and then mark it ready.

Also, if a leak is not found then at least we could get the test coverage for it.

@CaseyCarter
Copy link
Contributor

Closing the PR since there's nothing here to review. Feel free to ask questions or solicit comments about your intended approach in #4418, or to open another PR when you have potential changes to merge or to get comment on.

@CaseyCarter CaseyCarter closed this Oct 1, 2024
@Arup-Chauhan
Copy link
Contributor Author

Closing the PR since there's nothing here to review. Feel free to ask questions or solicit comments about your intended approach in #4418, or to open another PR when you have potential changes to merge or to get comment on.

Understood @CaseyCarter!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants