192502 2018-12-07 10:33:56 -0800 Ignore false-positive leaks under bmalloc::Heap::Heap 2019-02-19 19:04:28 -0800 1 1 1 Unclassified WebKit Tools / Tests WebKit Local Build Unspecified Unspecified RESOLVED CONFIGURATION CHANGED InRadar P2 Normal --- 1 ddkilzer ddkilzer ews-watchlist fpizlo ggaren glenn joepeck lforschler msaboff simon.fraser thorton webkit-bug-importer ysuzuki oldest_to_newest 1486031 0 ddkilzer 2018-12-07 10:33:56 -0800 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] 1486033 1 webkit-bug-importer 2018-12-07 10:34:44 -0800 <rdar://problem/46557903> 1486041 2 356821 ddkilzer 2018-12-07 10:42:47 -0800 Created attachment 356821 Patch v1 1486042 3 ap 2018-12-07 10:43:52 -0800 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. 1486043 4 356821 ap 2018-12-07 10:44:11 -0800 Comment on attachment 356821 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. 1486070 5 ddkilzer 2018-12-07 11:12:07 -0800 (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)? 1487551 6 ddkilzer 2018-12-13 04:03:00 -0800 (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). 1507967 7 ddkilzer 2019-02-19 19:04:28 -0800 I also wonder if r241789 for Bug 194811 improved this prior to the latest dev tools. 356821 2018-12-07 10:42:47 -0800 2019-02-19 19:04:02 -0800 Patch v1 bug-192502-20181207104246.patch text/plain 1422 ddkilzer U3VidmVyc2lvbiBSZXZpc2lvbjogMjM4OTQ4CmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggOGExODBlNjM0NTU1ZDM4YjM1Yzc5YjVjYzMzOTM1YzM1 ZGI0ZTYxMC4uNmVkM2IwNGEyNzM5Y2I0MmY4MGRiYTA2ZmEzMDRjYzdhNzY0ZDQ4NiAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1 IEBACisyMDE4LTEyLTA3ICBEYXZpZCBLaWx6ZXIgIDxkZGtpbHplckBhcHBsZS5jb20+CisKKyAg ICAgICAgSWdub3JlIGZhbHNlLXBvc2l0aXZlIGxlYWtzIHVuZGVyIGJtYWxsb2M6OkhlYXA6Okhl YXAKKyAgICAgICAgPGh0dHBzOi8vd2Via2l0Lm9yZy9iLzE5MjUwMj4KKyAgICAgICAgPHJkYXI6 Ly9wcm9ibGVtLzQ2NTU3OTAzPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEp LgorCisgICAgICAgICogU2NyaXB0cy93ZWJraXRweS9wb3J0L2xlYWtkZXRlY3Rvci5weToKKyAg ICAgICAgKExlYWtEZXRlY3Rvci5fY2FsbHN0YWNrc190b19leGNsdWRlX2Zyb21fbGVha3MpOiBF eGNsdWRlIGxlYWtzCisgICAgICAgIHdpdGggc3RhY2tzIGNvbnRhaW5pbmcgJ2JtYWxsb2M6Okhl YXA6OkhlYXAnLgorCiAyMDE4LTEyLTA2ICBEYXZpZCBLaWx6ZXIgIDxkZGtpbHplckBhcHBsZS5j b20+CiAKICAgICAgICAgSW5qZWN0ZWQgYnVuZGxlIGZvciBXZWJLaXRUZXN0UnVubmVyIGxlYWtz IFdLVHlwZVJlZiBvYmplY3RzCmRpZmYgLS1naXQgYS9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L3Bv cnQvbGVha2RldGVjdG9yLnB5IGIvVG9vbHMvU2NyaXB0cy93ZWJraXRweS9wb3J0L2xlYWtkZXRl Y3Rvci5weQppbmRleCBkMDdlMDk1OWU5MGEzNTRhNTBjNWRlZGQyMmJhOTFlMmNmZDc0OWMyLi5j OTY3YWQ3NWE2ZGE3OTJkMzcyMDMyZjdiYmZmMDQ1ZTViODBkZmE5IDEwMDY0NAotLS0gYS9Ub29s cy9TY3JpcHRzL3dlYmtpdHB5L3BvcnQvbGVha2RldGVjdG9yLnB5CisrKyBiL1Rvb2xzL1Njcmlw dHMvd2Via2l0cHkvcG9ydC9sZWFrZGV0ZWN0b3IucHkKQEAgLTYwLDYgKzYwLDcgQEAgY2xhc3Mg TGVha0RldGVjdG9yKG9iamVjdCk6CiAgICAgICAgICAgICAnQ0lEZXZpY2VNYW5hZ2VyU3RhcnRN b25pdG9yaW5nJywgIyA8cmRhcjovL3Byb2JsZW0vMzU3MTEwNTI+CiAgICAgICAgICAgICAnTlNT cGVsbENoZWNrZXIgaW5pdCcsICMgPHJkYXI6Ly9wcm9ibGVtLzM1NDM0NjE1PgogICAgICAgICAg ICAgJ05TQ29sb3IgY29udHJvbEhpZ2hsaWdodENvbG9yJywgIyA8cmRhcjovL3Byb2JsZW0vMzU4 MTYzMzI+CisgICAgICAgICAgICAnYm1hbGxvYzo6SGVhcDo6SGVhcCcsICMgPHJkYXI6Ly9wcm9i bGVtLzQwMzA1OTk0PgogICAgICAgICBdCiAgICAgICAgIHJldHVybiBjYWxsc3RhY2tzCiAK