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 memory leak (reachable) in ThreadIdToThreadLocals (Platform MS Windows) #757

Closed
wants to merge 1 commit into from

Conversation

klugier
Copy link

@klugier klugier commented Apr 13, 2016

Hello,

I am trying to port gtest to U++ bazar project (www.ultimatepp.org) , but it's memory dectors shows that there are memory leaks on MS Windows. I fount that one of this leaks comes from static ThreadLocalRegistryImpl method "ThreadIdToThreadLocals* GetThreadLocalsMapLocked()" - gtest-port.cpp - line 532. Below is the original implementation:

  // Returns map of thread local instances.
  static ThreadIdToThreadLocals* GetThreadLocalsMapLocked() {
    mutex_.AssertHeld();
    static ThreadIdToThreadLocals* map = new ThreadIdToThreadLocals;
    return map;
  }

As you can see the problem here is that this static variable is never deleted in the gtest code. The solution here is simply, instead of allocating on stack let's allocate this on heap:

  // Returns map of thread local instances.
  static ThreadIdToThreadLocals* GetThreadLocalsMapLocked() {
    mutex_.AssertHeld();
    static ThreadIdToThreadLocals map;
    return ↦
  }

In this solution destructor of ThreadIdToThreadLocals will be called when all static variable should be cleaned.

Sincerely,
Zbigniew Rębacz


Discussion in #692

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@klugier klugier changed the title Fix memory leak in ThreadIdToThreadLocals Fix memory leak (reachable) in ThreadIdToThreadLocals Apr 13, 2016
@klugier klugier changed the title Fix memory leak (reachable) in ThreadIdToThreadLocals Fix memory leak (reachable) in ThreadIdToThreadLocals (Platform MS Windows) Apr 13, 2016
@klugier
Copy link
Author

klugier commented Apr 13, 2016

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@klugier
Copy link
Author

klugier commented Nov 1, 2017

The above solution leads to crash. So, I am glad nobody response to it for 1.5 year. However, I found that some folks reports the other solution. I hope the problem will be solved in the future.

In U++ GTest bazaar project we deal with it by ignoring this leak and everything working as expected without warnings.

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.

None yet

2 participants