1842860 - [macOS 10.12] Crash in [@ malloc_zone_malloc | _CFArrayReplaceValues]

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect, P3)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

24 crashes from 24 devices with Firefox 115.0 or 115.0.1 on macOS 10.12. Version 115 is the ESR branch, a fix for this is worth to consider.

Crash report: https://crash-stats.mozilla.org/report/index/6bcc439d-6522-4d71-916b-29d860230710

Reason: EXC_BAD_ACCESS / KERN_PROTECTION_FAILURE

Top 10 frames of crashing thread:

0  libsystem_malloc.dylib  malloc_zone_malloc  
1  CoreFoundation  _CFArrayReplaceValues  
2  CoreFoundation  CFArrayAppendValue  
3  CoreWLAN  Apple80211Get  
4  CoreWLAN  Apple80211CopyValue  
5  CoreWLAN  Apple80211GetPower  
6  CoreWLAN  -[CWInterface powerOn]  
7  CoreWLAN  -[CWInterface initWithInterfaceName:xpcConnection:legacyEventMonitoring:]  
8  CoreWLAN  +[CWInterface interfaceWithName:]  
9  CoreWLAN  +[CWInterface interface]  

Comment 1

[Haik Aftandilian [:haik]]

On bug 1776210, we increased the Wifi Monitor thread stack size for macOS 13 and later because we found CoreWLAN functions were allocating data structures over 200k in size on the stack causing stack overflows.

Comment 2

[David Parks [:handyman]]

That's probably very related to what's happening but the stack size fix is still in there. However, my changes also led to a circumstance where the scan task is dispatched while SpinningEventLoopUntil the last scan is done running the location update on the main thread. The first scan's objects should no longer be on the stack so it's still not totally clear but the fact that it is a second scan on the same stack that crashes each time doesn't sound like coincidence.

Comment 3

[David Parks [:handyman]]

Latest theory is that the new setup adds just enough to the stack to push us over the limit when it nests tasks (comment 2)... and only on OS 10.12. It's probably fragile enough that we should increase the stack size everywhere, but we could also limit it to OnSierraExactly or avoid the nested tasks -- the last option is being done in bug 1843119.

For comparison, this is the stack from comment 0:

Crashing Thread (55), Name: Wifi Monitor
Frame 	Module 	Signature 	Source 	Trust
0 	libsystem_malloc.dylib 	malloc_zone_malloc 		context
1 	CoreFoundation 	_CFArrayReplaceValues 		cfi
2 	CoreFoundation 	CFArrayAppendValue 		cfi
3 	CoreWLAN 	Apple80211Get 		cfi
4 	CoreWLAN 	Apple80211CopyValue 		cfi
5 	CoreWLAN 	Apple80211GetPower 		cfi
6 	CoreWLAN 	-[CWInterface powerOn] 		cfi
7 	CoreWLAN 	-[CWInterface(Private) initWithInterfaceName:xpcConnection:legacyEventMonitoring:] 		cfi
8 	CoreWLAN 	+[CWInterface interfaceWithName:] 		cfi
9 	CoreWLAN 	+[CWInterface interface] 		cfi
10 	XUL 	mozilla::WifiScannerImpl::GetAccessPointsFromWLAN(nsTArray<RefPtr<nsIWifiAccessPoint> >&) 	netwerk/wifi/mac/MacWifiScanner.mm:44 	cfi
11 	XUL 	nsWifiMonitor::DoScan() 	netwerk/wifi/nsWifiMonitor.cpp:334 	cfi
12 	XUL 	nsWifiMonitor::Scan(unsigned long long) 	netwerk/wifi/nsWifiMonitor.cpp:287 	cfi
13 	XUL 	mozilla::detail::RunnableMethodArguments<unsigned long long>::apply<nsWifiMonitor, void (nsWifiMonitor::*)(unsigned long long)>(nsWifiMonitor*, void (nsWifiMonitor::*)(unsigned long long))::{lambda(auto:1&&)#1}::operator()<StoreCopyPassByConstLRef<unsigned long long>&>(StoreCopyPassByConstLRef<unsigned long long>&) const 	xpcom/threads/nsThreadUtils.h:1164 	inlined
13 	XUL 	std::__1::__invoke[abi:v15006]<mozilla::detail::RunnableMethodArguments<unsigned long long>::apply<nsWifiMonitor, void (mozilla::detail::RunnableMethodArguments<unsigned long long>::apply::*)(unsigned long long)>(nsWifiMonitor*, void (mozilla::detail::RunnableMethodArguments<unsigned long long>::apply::*)(unsigned long long))::{lambda(auto:1&&)#1}, StoreCopyPassByConstLRef<unsigned long long>&>(void (mozilla::detail::RunnableMethodArguments<unsigned long long>::apply::*&&)(unsigned long long), StoreCopyPassByConstLRef<unsigned long long>&) 	/builds/worker/fetches/MacOSX13.3.sdk/usr/include/c++/v1/__functional/invoke.h:394 	inlined
13 	XUL 	std::__1::__apply_tuple_impl[abi:v15006]<mozilla::detail::RunnableMethodArguments<unsigned long long>::apply<nsWifiMonitor, void (mozilla::detail::RunnableMethodArguments<unsigned long long>::apply::*)(unsigned long long)>(nsWifiMonitor*, void (mozilla::detail::RunnableMethodArguments<unsigned long long>::apply::*)(unsigned long long))::{lambda(auto:1&&)#1}, std::__1::tuple<StoreCopyPassByConstLRef<unsigned long long> >&, (unsigned long)0>(void (mozilla::detail::RunnableMethodArguments<unsigned long long>::apply::*&&)(unsigned long long), mozilla::detail::RunnableMethodArguments<unsigned long long>::apply<nsWifiMonitor, void (mozilla::detail::RunnableMethodArguments<unsigned long long>::apply::*)(unsigned long long)>(nsWifiMonitor*, void (mozilla::detail::RunnableMethodArguments<unsigned long long>::apply::*)(unsigned long long))::{lambda(auto:1&&)#1}*&&, std::__1::__tuple_indices<((unsigned long)0)...>) 	/builds/worker/fetches/MacOSX13.3.sdk/usr/include/c++/v1/tuple:1789 	inlined
13 	XUL 	std::__1::apply[abi:v15006]<mozilla::detail::RunnableMethodArguments<unsigned long long>::apply<nsWifiMonitor, void (mozilla::detail::RunnableMethodArguments<unsigned long long>::apply::*)(unsigned long long)>(nsWifiMonitor*, void (mozilla::detail::RunnableMethodArguments<unsigned long long>::apply::*)(unsigned long long))::{lambda(auto:1&&)#1}, std::__1::tuple<StoreCopyPassByConstLRef<unsigned long long> >&>(void (mozilla::detail::RunnableMethodArguments<unsigned long long>::apply::*&&)(unsigned long long), mozilla::detail::RunnableMethodArguments<unsigned long long>::apply<nsWifiMonitor, void (mozilla::detail::RunnableMethodArguments<unsigned long long>::apply::*)(unsigned long long)>(nsWifiMonitor*, void (mozilla::detail::RunnableMethodArguments<unsigned long long>::apply::*)(unsigned long long))::{lambda(auto:1&&)#1}*&&) 	/builds/worker/fetches/MacOSX13.3.sdk/usr/include/c++/v1/tuple:1798 	inlined
13 	XUL 	mozilla::detail::RunnableMethodArguments<unsigned long long>::apply<nsWifiMonitor, void (nsWifiMonitor::*)(unsigned long long)>(nsWifiMonitor*, void (nsWifiMonitor::*)(unsigned long long)) 	xpcom/threads/nsThreadUtils.h:1162 	inlined
13 	XUL 	mozilla::detail::RunnableMethodImpl<nsWifiMonitor*, void (nsWifiMonitor::*)(unsigned long long), true, (mozilla::RunnableKind)0, unsigned long long>::Run() 	xpcom/threads/nsThreadUtils.h:1213 	cfi
14 	XUL 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp:1234 	cfi
15 	XUL 	NS_ProcessNextEvent(nsIThread*, bool) 	xpcom/threads/nsThreadUtils.cpp:479 	inlined
15 	XUL 	mozilla::SpinEventLoopUntil<(mozilla::ProcessFailureBehavior)1, nsThreadSyncDispatch::SpinEventLoopUntilComplete(nsTSubstring<char> const&)::{lambda()#1}>(nsTSubstring<char> const&, nsThreadSyncDispatch::SpinEventLoopUntilComplete(nsTSubstring<char> const&)::{lambda()#1}&&, nsIThread*) 	xpcom/threads/SpinEventLoopUntil.h:176 	inlined
15 	XUL 	nsThreadSyncDispatch::SpinEventLoopUntilComplete(nsTSubstring<char> const&) 	xpcom/threads/nsThreadSyncDispatch.h:32 	inlined
15 	XUL 	NS_DispatchAndSpinEventLoopUntilComplete(nsTSubstring<char> const&, nsIEventTarget*, already_AddRefed<nsIRunnable>) 	xpcom/threads/nsThreadUtils.cpp:766 	cfi
16 	XUL 	nsWifiMonitor::Scan(unsigned long long) 	netwerk/wifi/nsWifiMonitor.cpp:298 	cfi
17 	XUL 	mozilla::detail::RunnableMethodArguments<unsigned long long>::apply<nsWifiMonitor, void (nsWifiMonitor::*)(unsigned long long)>(nsWifiMonitor*, void (nsWifiMonitor::*)(unsigned long long))::{lambda(auto:1&&)#1}::operator()<StoreCopyPassByConstLRef<unsigned long long>&>(StoreCopyPassByConstLRef<unsigned long long>&) const 	xpcom/threads/nsThreadUtils.h:1164 	inlined
17 	XUL 	std::__1::__invoke[abi:v15006]<mozilla::detail::RunnableMethodArguments<unsigned long long>::apply<nsWifiMonitor, void (mozilla::detail::RunnableMethodArguments<unsigned long long>::apply::*)(unsigned long long)>(nsWifiMonitor*, void (mozilla::detail::RunnableMethodArguments<unsigned long long>::apply::*)(unsigned long long))::{lambda(auto:1&&)#1}, StoreCopyPassByConstLRef<unsigned long long>&>(void (mozilla::detail::RunnableMethodArguments<unsigned long long>::apply::*&&)(unsigned long long), StoreCopyPassByConstLRef<unsigned long long>&) 	/builds/worker/fetches/MacOSX13.3.sdk/usr/include/c++/v1/__functional/invoke.h:394 	inlined
17 	XUL 	std::__1::__apply_tuple_impl[abi:v15006]<mozilla::detail::RunnableMethodArguments<unsigned long long>::apply<nsWifiMonitor, void (mozilla::detail::RunnableMethodArguments<unsigned long long>::apply::*)(unsigned long long)>(nsWifiMonitor*, void (mozilla::detail::RunnableMethodArguments<unsigned long long>::apply::*)(unsigned long long))::{lambda(auto:1&&)#1}, std::__1::tuple<StoreCopyPassByConstLRef<unsigned long long> >&, (unsigned long)0>(void (mozilla::detail::RunnableMethodArguments<unsigned long long>::apply::*&&)(unsigned long long), mozilla::detail::RunnableMethodArguments<unsigned long long>::apply<nsWifiMonitor, void (mozilla::detail::RunnableMethodArguments<unsigned long long>::apply::*)(unsigned long long)>(nsWifiMonitor*, void (mozilla::detail::RunnableMethodArguments<unsigned long long>::apply::*)(unsigned long long))::{lambda(auto:1&&)#1}*&&, std::__1::__tuple_indices<((unsigned long)0)...>) 	/builds/worker/fetches/MacOSX13.3.sdk/usr/include/c++/v1/tuple:1789 	inlined
17 	XUL 	std::__1::apply[abi:v15006]<mozilla::detail::RunnableMethodArguments<unsigned long long>::apply<nsWifiMonitor, void (mozilla::detail::RunnableMethodArguments<unsigned long long>::apply::*)(unsigned long long)>(nsWifiMonitor*, void (mozilla::detail::RunnableMethodArguments<unsigned long long>::apply::*)(unsigned long long))::{lambda(auto:1&&)#1}, std::__1::tuple<StoreCopyPassByConstLRef<unsigned long long> >&>(void (mozilla::detail::RunnableMethodArguments<unsigned long long>::apply::*&&)(unsigned long long), mozilla::detail::RunnableMethodArguments<unsigned long long>::apply<nsWifiMonitor, void (mozilla::detail::RunnableMethodArguments<unsigned long long>::apply::*)(unsigned long long)>(nsWifiMonitor*, void (mozilla::detail::RunnableMethodArguments<unsigned long long>::apply::*)(unsigned long long))::{lambda(auto:1&&)#1}*&&) 	/builds/worker/fetches/MacOSX13.3.sdk/usr/include/c++/v1/tuple:1798 	inlined
17 	XUL 	mozilla::detail::RunnableMethodArguments<unsigned long long>::apply<nsWifiMonitor, void (nsWifiMonitor::*)(unsigned long long)>(nsWifiMonitor*, void (nsWifiMonitor::*)(unsigned long long)) 	xpcom/threads/nsThreadUtils.h:1162 	inlined
17 	XUL 	mozilla::detail::RunnableMethodImpl<nsWifiMonitor*, void (nsWifiMonitor::*)(unsigned long long), true, (mozilla::RunnableKind)0, unsigned long long>::Run() 	xpcom/threads/nsThreadUtils.h:1213 	cfi
18 	XUL 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp:1234 	cfi
19 	XUL 	NS_ProcessNextEvent(nsIThread*, bool) 	xpcom/threads/nsThreadUtils.cpp:479 	inlined
19 	XUL 	mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp:330 	cfi
20 	XUL 	MessageLoop::RunInternal() 	ipc/chromium/src/base/message_loop.cc:368 	inlined
20 	XUL 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc:361 	inlined
20 	XUL 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc:343 	cfi
21 	XUL 	nsThread::ThreadFunc(void*) 	xpcom/threads/nsThread.cpp:391 	cfi
22 	libnss3.dylib 	_pt_root 	nsprpub/pr/src/pthreads/ptthread.c:201 	cfi
23 	libsystem_pthread.dylib 	_pthread_body 		cfi
24 	libsystem_pthread.dylib 	_pthread_start 		cfi
25 	libsystem_pthread.dylib 	thread_start 		cfi

and a stack for the old crash is at bug 1776210 comment 5. Some things have been added. I'm tempted to increase the stack size for ESR since that is less advanced than bug 1843119.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

Note that 10.12 isn't supported for 116+

Comment 5

[Haik Aftandilian [:haik]]

I have 10.12.6 on an older MacBook Pro and was able to reproduce the crash with Firefox 115.0.2. I had maps.google.com open and after the computer went to sleep I later found the browser crashed. I experimented with WiFi off/on, but don't have reliable repro steps yet.

https://crash-stats.mozilla.org/report/index/0803b29c-ce90-451a-bdb2-609810230714

Comment 6

[David Parks [:handyman]]

Attachment: Bug 1842860: Increase stack size in WifiMonitor on MacOS r=haik!

We have been seeing the stack issue from bug 1776210, but on Sierra.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6cbe2549217c
Increase stack size in WifiMonitor on MacOS r=haik,necko-reviewers,kershaw

Comment 8

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/6cbe2549217c

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:handyman, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox116 to wontfix.

For more information, please visit BugBot documentation.

Comment 10

[David Parks [:handyman]]

Comment on attachment 9343954 [details]
Bug 1842860: Increase stack size in WifiMonitor on MacOS r=haik!

Beta/Release Uplift Approval Request

• User impact if declined: None that we know of. The crashes were seen in MacOS 10.12, which is not supported after Firefox 115. Outside of ESR, the patch is a safeguard against future OS changes, as we appear to be right on the stack-size boundary.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Increasing the stack size by a few kB on the wifi monitoring thread is not controversial.
• String changes made/needed: N/A
• Is Android affected?: No

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: MacOS 10.12 crashes happen sometimes when using geolocation. The fix, of increasing the stack size, is basic.
• User impact if declined: Crashes for MacOS 10.12 users using geolocation. Potentially, crashes for other MacOS versions in the future.
• Fix Landed on Version: 117
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Increasing the stack size by a few kB on the wifi monitoring thread is not controversial.

Comment 11

[David Parks [:handyman]]

Comment on attachment 9343954 [details]
Bug 1842860: Increase stack size in WifiMonitor on MacOS r=haik!

On second thought, lets skip beta uplift.

Comment 12

[Dianna Smith [:diannaS]]

Comment on attachment 9343954 [details]
Bug 1842860: Increase stack size in WifiMonitor on MacOS r=haik!

Approved for 115.1esr

Comment 13

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/06ae187d51c6