Memory leak in ThreadLocalRegistryImpl::GetThreadLocalsMapLocked() on Windows

[klugier]

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

[nachokruss]

That would make things worse as local variables are destroyed after GetThreadLocalsMapLocked returns.

[dawikur]

@nachokruss local yes, but static are not.

[nachokruss]

Aha. You are right didn't see it was static, maybe put in a pull request then?

[klugier]

Thanks for comments. I will create pull request in near future.

[coreykosak]

This is not a leak per se (where a leak is defined as "allocated memory
that has become inaccessible from any pointer"), but rather a failure to
destroy all allocated objects at program shutdown.

Put another way, that object is always reachable, so it's not a leak.

So far as I am aware (maybe the gtest owners want to comment more
authoritatively), I don't think we care about carefully destroying every
last object at shutdown.

Is there an argument (other than hygeine / warnings from the leak detector
you are using) why this design choice hurts your project?

On Tue, Jan 26, 2016 at 2:28 PM, Zbigniew Rębacz [email protected]
wrote:

Thanks for discussion guys. I will create pull request in near future.

—
Reply to this email directly or view it on GitHub
#692 (comment).

[MrSapps]

Its annoying to find real leaks with lots of on purpose leaked objects :)

[gennadiycivil]

Should be addressed by #1142