Bug 189853 – [Cocoa] Avoid false report leaks under PerProcess

• Status: RESOLVED
• Resolution: CONFIGURATION CHANGED
• Component: bmalloc

[Joseph Pecoraro]

Avoid false report leaks under PerProcess.

Processes with bmalloc are often seeing the following instances show up in `leaks` output:

    1 (128 bytes) ROOT LEAK: <OS_dispatch_source 0x104304720> [128]
    1 (96 bytes) ROOT LEAK: <std::__1::__shared_ptr_emplace<std::__1::mutex, std::__1::allocator<std::__1::mutex> > 0x104302000> [96]
    1 (96 bytes) ROOT LEAK: <std::__1::__shared_ptr_emplace<std::__1::mutex, std::__1::allocator<std::__1::mutex> > 0x104304880> [96]
    1 (96 bytes) ROOT LEAK: <std::__1::__shared_ptr_emplace<std::__1::mutex, std::__1::allocator<std::__1::mutex> > 0x1043048e0> [96]

With allocation stack:

    STACK OF 1 INSTANCE OF 'ROOT LEAK: <std::__1::__shared_ptr_emplace<std::__1::mutex, std::__1::allocator<std::__1::mutex> >>':
    [thread 0x16dd67000]:
    ...
    7   JavaScriptCore                        0x1af1202b4 WTF::fastZeroedMalloc(unsigned long) + 124
    6   JavaScriptCore                        0x1af28d688 bmalloc::Cache::allocateSlowCaseNullCache(bmalloc::HeapKind, unsigned long) + 112
    5   JavaScriptCore                        0x1af28da5c bmalloc::PerHeapKindBase<bmalloc::Cache>::PerHeapKindBase<>() + 44
    4   JavaScriptCore                        0x1af28d4c4 bmalloc::PerProcess<bmalloc::PerHeapKind<bmalloc::Heap> >::getSlowCase() + 200
    3   JavaScriptCore                        0x1af290434 bmalloc::Heap::Heap(bmalloc::HeapKind, std::__1::lock_guard<bmalloc::Mutex>&) + 532
    2   libc++abi.dylib                       0x1a68236d4 operator new(unsigned long) + 44
    1   libsystem_malloc.dylib                0x1a716c8a0 malloc + 32
    0   libsystem_malloc.dylib                0x1a716b4e8 malloc_zone_malloc + 200 

It turns out that these are false reports since the `leaks` tool doesn't know how to traverse the allocations in different malloc zones. PerProcess.cpp uses uses vmAllocate() directly, which makes an mmap with the BMALLOC tag. PerProcess then inserts some objects with system malloc allocations inside of it. Specifically bmalloc::Heap has a std::condition_variable_any which inside has a std::shared_ptr<std::mutex> (which evidently performs an allocation with the system malloc). `leaks` doesn't know how to interpret data inside of BMALLOC tagged allocations, so it doesn't end up following pointers to know that the shared_ptr is in fact referenced.

One easy solution to avoid the false reports is to make the PerProcess vmAllocate a system malloc allocation, which `leaks` will know how to traverse. Since this is a single allocation it seems reasonable to avoid the false reports.

[Joseph Pecoraro]

<rdar://problem/40305994>

[Joseph Pecoraro]

Created attachment 350415 [details]
[PATCH] Proposed Fix

[Geoffrey Garen]

Comment on attachment 350415 [details]
[PATCH] Proposed Fix

This is kinda weird. Can we just have our heap tool scanner scan the PerProcess<Heap> object instead?

[Joseph Pecoraro]

(In reply to Geoffrey Garen from comment #3)
> Comment on attachment 350415 [details]
> [PATCH] Proposed Fix
> 
> This is kinda weird. Can we just have our heap tool scanner scan the
> PerProcess<Heap> object instead?

The allocation that needs to be scanned is the PerProcess<T> allocated inside of the PerProcessData* s_table allocations.

I'm not sure why the heap scanning doesn't follow the references to eventually find the system allocations when these are bmalloc tagged but do find it without the bmalloc tag. I was originally thinking of (<rdar://problem/37468980>) but I don't think that applies here. Let me follow up with those more familiar with heap scanning.

[Joseph Pecoraro]

*** Bug 191059 has been marked as a duplicate of this bug. ***

[Joseph Pecoraro]

Comment on attachment 350415 [details]
[PATCH] Proposed Fix

Removing review flag.

It seems this will be getting addressed by a tools change.

I'll close this in the meantime.