Bug 192502

Summary: Ignore false-positive leaks under bmalloc::Heap::Heap
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: Tools / TestsAssignee: David Kilzer (:ddkilzer) <ddkilzer>
Status: RESOLVED CONFIGURATION CHANGED    
Severity: Normal CC: ews-watchlist, fpizlo, ggaren, glenn, joepeck, lforschler, msaboff, simon.fraser, thorton, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch v1 none

David Kilzer (:ddkilzer)
Reported 2018-12-07 10:33:56 PST
We should ignore false-positive leaks under bmalloc::Heap::Heap. Eventually the `leaks` tool will handle this, but until then we need to skip these leaks. STACK OF 1 INSTANCE OF 'ROOT LEAK: <OS_dispatch_source>': [thread 0x16dd67000]: 29 libsystem_pthread.dylib 0x1a71ac068 start_wqthread + 4 28 libsystem_pthread.dylib 0x1a71ac24c _pthread_wqthread + 472 27 libdispatch.dylib 0x1a6fbdd54 _dispatch_worker_thread2 + 116 26 libdispatch.dylib 0x1a6fbd4f8 _dispatch_root_queue_drain + 344 25 libdispatch.dylib 0x1a6fb13c0 _dispatch_queue_override_invoke + 364 24 libdispatch.dylib 0x1a6fb63e8 _dispatch_lane_invoke$VARIANT$armv81 + 536 23 libdispatch.dylib 0x1a6fbc4c0 _dispatch_lane_concurrent_drain + 856 22 libdispatch.dylib 0x1a6f79808 _dispatch_client_callout + 16 21 libdispatch.dylib 0x1a6f79848 _dispatch_call_block_and_release + 24 20 com.apple.Safari.Shared 0x1ba630abc -[WBSSiteMetadataImageCache _internalSetUpImageCache] + 468 19 com.apple.Safari.Shared 0x1ba631774 -[WBSSiteMetadataImageCache _dispatchDiskAccessBlock:] + 96 18 com.apple.Safari.Shared 0x1ba51635c SafariShared::SuddenTerminationDisabler::SuddenTerminationDisabler(NSString*) + 64 17 com.apple.Safari.Shared 0x1ba4fd5c8 SafariShared::SuddenTerminationDisabler::disableSuddenTermination() + 92 16 libdispatch.dylib 0x1a6fbb154 _dispatch_lane_barrier_sync_invoke_and_complete + 56 15 libdispatch.dylib 0x1a6f79808 _dispatch_client_callout + 16 14 com.apple.Safari.Shared 0x1ba4fd70c invocation function for block in SafariShared::SuddenTerminationDisabler::disableSuddenTermination() + 236 13 com.apple.Safari.Shared 0x1ba4fdc88 WTF::HashTable<SafariShared::SuddenTerminationDisabler*, SafariShared::SuddenTerminationDisabler*, WTF::IdentityExtractor, WTF::PtrHash<SafariShared::SuddenTerminationDisabler*>, WTF::HashTraits<SafariShared::SuddenTerminationDisabler*>, WTF::HashTraits<SafariShared::SuddenTerminationDisabler*> >::add(SafariShared::SuddenTerminationDisabler*&&) + 88 12 com.apple.Safari.Shared 0x1ba4fde14 WTF::HashTable<SafariShared::SuddenTerminationDisabler*, SafariShared::SuddenTerminationDisabler*, WTF::IdentityExtractor, WTF::PtrHash<SafariShared::SuddenTerminationDisabler*>, WTF::HashTraits<SafariShared::SuddenTerminationDisabler*>, WTF::HashTraits<SafariShared::SuddenTerminationDisabler*> >::rehash(unsigned int, SafariShared::SuddenTerminationDisabler**) + 56 11 JavaScriptCore 0x1af1202b4 WTF::fastZeroedMalloc(unsigned long) + 124 10 JavaScriptCore 0x1af28d688 bmalloc::Cache::allocateSlowCaseNullCache(bmalloc::HeapKind, unsigned long) + 112 9 JavaScriptCore 0x1af28da5c bmalloc::PerHeapKindBase<bmalloc::Cache>::PerHeapKindBase<>() + 44 8 JavaScriptCore 0x1af28d4b0 bmalloc::PerProcess<bmalloc::PerHeapKind<bmalloc::Heap> >::getSlowCase() + 180 7 JavaScriptCore 0x1af290594 bmalloc::Heap::Heap(bmalloc::HeapKind, std::__1::lock_guard<bmalloc::Mutex>&) + 884 6 JavaScriptCore 0x1af28cdec bmalloc::PerProcess<bmalloc::Scavenger>::getSlowCase() + 140 5 JavaScriptCore 0x1af29632c bmalloc::Scavenger::Scavenger(std::__1::lock_guard<bmalloc::Mutex>&) + 212 4 libdispatch.dylib 0x1a6fbfe70 dispatch_source_create$VARIANT$armv81 + 56 3 libdispatch.dylib 0x1a6f79880 _os_object_alloc_realized + 40 2 libobjc.A.dylib 0x1a684eb48 class_createInstance + 72 1 libsystem_malloc.dylib 0x1a716c708 calloc + 40 0 libsystem_malloc.dylib 0x1a716c7e4 malloc_zone_calloc + 184 ==== 1 (128 bytes) ROOT LEAK: <OS_dispatch_source 0x11ff74740> [128] STACK OF 1 INSTANCE OF 'ROOT LEAK: <std::__1::__shared_ptr_emplace<std::__1::mutex, std::__1::allocator<std::__1::mutex> >>': [thread 0x16dd67000]: 25 libsystem_pthread.dylib 0x1a71ac068 start_wqthread + 4 24 libsystem_pthread.dylib 0x1a71ac24c _pthread_wqthread + 472 23 libdispatch.dylib 0x1a6fbdd54 _dispatch_worker_thread2 + 116 22 libdispatch.dylib 0x1a6fbd4f8 _dispatch_root_queue_drain + 344 21 libdispatch.dylib 0x1a6fb13c0 _dispatch_queue_override_invoke + 364 20 libdispatch.dylib 0x1a6fb63e8 _dispatch_lane_invoke$VARIANT$armv81 + 536 19 libdispatch.dylib 0x1a6fbc4c0 _dispatch_lane_concurrent_drain + 856 18 libdispatch.dylib 0x1a6f79808 _dispatch_client_callout + 16 17 libdispatch.dylib 0x1a6f79848 _dispatch_call_block_and_release + 24 16 com.apple.Safari.Shared 0x1ba630abc -[WBSSiteMetadataImageCache _internalSetUpImageCache] + 468 15 com.apple.Safari.Shared 0x1ba631774 -[WBSSiteMetadataImageCache _dispatchDiskAccessBlock:] + 96 14 com.apple.Safari.Shared 0x1ba51635c SafariShared::SuddenTerminationDisabler::SuddenTerminationDisabler(NSString*) + 64 13 com.apple.Safari.Shared 0x1ba4fd5c8 SafariShared::SuddenTerminationDisabler::disableSuddenTermination() + 92 12 libdispatch.dylib 0x1a6fbb154 _dispatch_lane_barrier_sync_invoke_and_complete + 56 11 libdispatch.dylib 0x1a6f79808 _dispatch_client_callout + 16 10 com.apple.Safari.Shared 0x1ba4fd70c invocation function for block in SafariShared::SuddenTerminationDisabler::disableSuddenTermination() + 236 9 com.apple.Safari.Shared 0x1ba4fdc88 WTF::HashTable<SafariShared::SuddenTerminationDisabler*, SafariShared::SuddenTerminationDisabler*, WTF::IdentityExtractor, WTF::PtrHash<SafariShared::SuddenTerminationDisabler*>, WTF::HashTraits<SafariShared::SuddenTerminationDisabler*>, WTF::HashTraits<SafariShared::SuddenTerminationDisabler*> >::add(SafariShared::SuddenTerminationDisabler*&&) + 88 8 com.apple.Safari.Shared 0x1ba4fde14 WTF::HashTable<SafariShared::SuddenTerminationDisabler*, SafariShared::SuddenTerminationDisabler*, WTF::IdentityExtractor, WTF::PtrHash<SafariShared::SuddenTerminationDisabler*>, WTF::HashTraits<SafariShared::SuddenTerminationDisabler*>, WTF::HashTraits<SafariShared::SuddenTerminationDisabler*> >::rehash(unsigned int, SafariShared::SuddenTerminationDisabler**) + 56 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 0x1af28d4b0 bmalloc::PerProcess<bmalloc::PerHeapKind<bmalloc::Heap> >::getSlowCase() + 180 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 ==== 1 (96 bytes) ROOT LEAK: <std::__1::__shared_ptr_emplace<std::__1::mutex, std::__1::allocator<std::__1::mutex> > 0x11fd5e620> [96]
Attachments
Patch v1 (1.39 KB, patch)
2018-12-07 10:42 PST, David Kilzer (:ddkilzer) 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2018-12-07 10:34:44 PST
<rdar://problem/46557903>
David Kilzer (:ddkilzer)
Comment 2 2018-12-07 10:42:47 PST
Created attachment 356821 [details] Patch v1
Alexey Proskuryakov
Comment 3 2018-12-07 10:43:52 PST
What exactly is the tools bug here? run-webkit-tests --leaks sets the MallocStackLogging environment variable, which should make bmalloc disable any functionality that's still incompatible with the leaks tool. So I think that this is a bmalloc bug.
Alexey Proskuryakov
Comment 4 2018-12-07 10:44:11 PST
Comment on attachment 356821 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=356821&action=review > Tools/Scripts/webkitpy/port/leakdetector.py:63 > + 'bmalloc::Heap::Heap', # <rdar://problem/40305994> As just commented, I don't see how this is the right approach.
David Kilzer (:ddkilzer)
Comment 5 2018-12-07 11:12:07 PST
(In reply to Alexey Proskuryakov from comment #3) > What exactly is the tools bug here? run-webkit-tests --leaks sets the > MallocStackLogging environment variable, which should make bmalloc disable > any functionality that's still incompatible with the leaks tool. This is fixed in a future version of the leaks tool by <rdar://problem/40305994>. The bug apparently had to do with the way either fastMalloc or bmalloc tagged its allocated memory. > So I think that this is a bmalloc bug. This change is to ignore these false-positive leaks until the new leaks tool (with the fix) is widely available as "bmalloc::Heap::Heap" will ignore both of these stacks, which are allocating memory using malloc (see stack frame 0): STACK OF 1 INSTANCE OF 'ROOT LEAK: <OS_dispatch_source>': [thread 0x16dd67000]: [...] 7 JavaScriptCore 0x1af290594 bmalloc::Heap::Heap(bmalloc::HeapKind, std::__1::lock_guard<bmalloc::Mutex>&) + 884 6 JavaScriptCore 0x1af28cdec bmalloc::PerProcess<bmalloc::Scavenger>::getSlowCase() + 140 5 JavaScriptCore 0x1af29632c bmalloc::Scavenger::Scavenger(std::__1::lock_guard<bmalloc::Mutex>&) + 212 4 libdispatch.dylib 0x1a6fbfe70 dispatch_source_create$VARIANT$armv81 + 56 3 libdispatch.dylib 0x1a6f79880 _os_object_alloc_realized + 40 2 libobjc.A.dylib 0x1a684eb48 class_createInstance + 72 1 libsystem_malloc.dylib 0x1a716c708 calloc + 40 0 libsystem_malloc.dylib 0x1a716c7e4 malloc_zone_calloc + 184 ==== 1 (128 bytes) ROOT LEAK: <OS_dispatch_source 0x11ff74740> [128] STACK OF 1 INSTANCE OF 'ROOT LEAK: <std::__1::__shared_ptr_emplace<std::__1::mutex, std::__1::allocator<std::__1::mutex> >>': [thread 0x16dd67000]: [...] 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 ==== 1 (96 bytes) ROOT LEAK: <std::__1::__shared_ptr_emplace<std::__1::mutex, std::__1::allocator<std::__1::mutex> > 0x11fd5e620> [96] Or are you saying that the fix for <rdar://problem/40305994> was the wrong way to fix this, and the change should be in bmalloc instead to disable the bmalloc code and to call malloc directly from WTF::fastZeroedMalloc() (since the Scavenger isn't even used when malloc is used to allocate memory)?
David Kilzer (:ddkilzer)
Comment 6 2018-12-13 04:03:00 PST
(In reply to Alexey Proskuryakov from comment #3) > What exactly is the tools bug here? run-webkit-tests --leaks sets the > MallocStackLogging environment variable, which should make bmalloc disable > any functionality that's still incompatible with the leaks tool. > > So I think that this is a bmalloc bug. I did some testing: - Enabling USE_SYSTEM_MALLOC at build time makes this go away since these objects are never created. However, that requires a separate build and some local hacks to make it compile, and it's even further from the configuration that customers use because bmalloc is not used at all. (Maybe using system malloc instead of bmalloc is far enough from a customer configuration that this doesn't matter, though?) - These false-positive leaks only affect bmalloc itself, not the memory bmalloc allocates on behalf of other code, so this change will only ignore these 4 leaks (there are 3 instances of the mutex mentioned in Comment #0) in each com.apple.WebKit.* process and the UI process. - There may not be another chokepoint for bmalloc vs. system malloc that both (a) doesn't impact customer performance, and (b) eliminates these allocations. However, doing nothing means updated tools will eventually stop reporting these leaks anyway, so let's just move this bug to config changed since it will now never land, and it only affects a person or a bot running leaks with modern WebKit processes (which isn't supported with `run-webkit-tests --leaks` yet).
David Kilzer (:ddkilzer)
Comment 7 2019-02-19 19:04:28 PST
I also wonder if r241789 for Bug 194811 improved this prior to the latest dev tools.
Note You need to log in before you can comment on or make changes to this bug.