1984088 - Crash in [@ mozilla::Maybe<T>::value | mozilla::places::AsyncGetFaviconForPageRunnable::Run::<T>::operator()]

Status: RESOLVED FIXED

(Toolkit :: Places, defect, P1)

Description

[Donal Meehan [:dmeehan]]

Crash report: https://crash-stats.mozilla.org/report/index/1f38b1e9-007b-47a4-8201-d26180250820

MOZ_CRASH Reason:

MOZ_RELEASE_ASSERT(isSome())

Top 10 frames:

0  xul.dll  mozilla::Maybe<mozilla::places::ConcurrentConnection*>::value() const  mfbt/Maybe.h:924
0  xul.dll  mozilla::places::AsyncGetFaviconForPageRunnable::Run::<lambda_9>::operator()(...  toolkit/components/places/FaviconHelpers.cpp:933
0  xul.dll  mozilla::places::AsyncGetFaviconForPageRunnable::Run()  toolkit/components/places/FaviconHelpers.cpp:925
1  xul.dll  nsThread::ProcessNextEvent(bool, bool*)  xpcom/threads/nsThread.cpp:1153
1  xul.dll  NS_ProcessNextEvent(nsIThread*, bool)  xpcom/threads/nsThreadUtils.cpp:480
2  xul.dll  mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*)  ipc/glue/MessagePump.cpp:299
3  xul.dll  MessageLoop::RunInternal()  ipc/chromium/src/base/message_loop.cc:374
3  xul.dll  MessageLoop::RunHandler()  ipc/chromium/src/base/message_loop.cc:367
4  xul.dll  MessageLoop::Run()  ipc/chromium/src/base/message_loop.cc:349
4  xul.dll  nsThread::ThreadFunc(void*)  xpcom/threads/nsThread.cpp:366

Comment 1

[Donal Meehan [:dmeehan]]

:mak, Bug 599980 looks like a regressor here.
Bug 1980992 is possibly related, added to the see also.

Comment 2

[Marco Bonardo [:mak]]

We are doing a null check here, but we're not properly handling Maybe, before extracting a value we must check if it contains something.

Comment 3

[Marco Bonardo [:mak]]

Attachment: Bug 1984088 - Check Maybe result in AsyncGetFaviconForPageRunnable to avoid crash. r=jteow

Comment 4

[Donal Meehan [:dmeehan]]

The patch is pending review, but adding a reminder that next week is the final week of beta for Fx143.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/44f30b96dfb2
https://hg.mozilla.org/integration/autoland/rev/afb3f23b3d68
Check Maybe result in AsyncGetFaviconForPageRunnable to avoid crash. r=jteow

Comment 6

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/afb3f23b3d68

Comment 7

[Marco Bonardo [:mak]]

Attachment: Bug 1984088 - Check Maybe result in AsyncGetFaviconForPageRunnable to avoid crash. r=jteow

Original Revision: https://phabricator.services.mozilla.com/D262160

Comment 8

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined: crash
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: not easily testable
• Risk associated with taking this patch: low
• Explanation of risk level: added value validity check, small change
• String changes made/needed: no
• Is Android affected?: no

Comment 9

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/802e40305564
https://hg.mozilla.org/releases/mozilla-beta/rev/7035758949aa