Crash in [@ mozilla::dom::DOMIntersectionObserver::Unobserve] with tampermonkey addon
Categories
(Core :: Layout, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr78 | --- | unaffected |
firefox88 | --- | wontfix |
firefox89 | + | fixed |
firefox90 | --- | fixed |
People
(Reporter: aryx, Assigned: emilio)
Details
(Keywords: crash)
Crash Data
Attachments
(2 files)
48 bytes,
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta-
RyanVM
:
approval-mozilla-release-
|
Details | Review |
1.76 KB,
patch
|
pascalc
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
~130 crashes for ~90 installs of Firefox 88.0/88.0.1
Code had landed in bug 1687358 for Firefox 87 but there are only single crash instances before Gecko 88.
Sean, can you check what changed related to this in Firefox 88?
Crash report: https://crash-stats.mozilla.org/report/index/ab6791ec-6a42-4897-a1db-f41c50210509
Reason: EXCEPTION_ACCESS_VIOLATION_READ
Top 10 frames of crashing thread:
0 xul.dll mozilla::dom::DOMIntersectionObserver::Unobserve dom/base/DOMIntersectionObserver.cpp:240
1 xul.dll mozilla::dom::HTMLImageElement::LazyLoadImageReachedViewport dom/html/HTMLImageElement.cpp:1307
2 xul.dll mozilla::dom::LazyLoadCallbackReachViewport dom/base/DOMIntersectionObserver.cpp:165
3 xul.dll mozilla::dom::DOMIntersectionObserver::Notify dom/base/DOMIntersectionObserver.cpp:751
4 xul.dll mozilla::dom::Document::NotifyIntersectionObservers dom/base/Document.cpp:15439
5 xul.dll mozilla::detail::RunnableMethodImpl< xpcom/threads/nsThreadUtils.h:1201
6 xul.dll mozilla::SchedulerGroup::Runnable::Run xpcom/threads/SchedulerGroup.cpp:143
7 xul.dll mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal xpcom/threads/TaskController.cpp:757
8 xul.dll mozilla::detail::RunnableFunction<`lambda at /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:135:7'>::Run xpcom/threads/nsThreadUtils.h:534
9 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp
Reporter | ||
Comment 1•3 years ago
|
||
There is a strong correlation with the Tampermonkey add-on and the issue hadn't been observed initially with the new Firefox 88.0 version. It might have started with the rollout of the latest Tampermonkey version: https://addons.mozilla.org/en-US/firefox/addon/tampermonkey/
Reporter | ||
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Comment 2•3 years ago
|
||
Hi, Tampermonkey dev here. Please tell me in case I can help to investigate the issue.
Comment 3•3 years ago
|
||
FWIW: Tampermonkey 4.13.6136 roll out started on 2021-04-30
2021-04-30: 9K+ updates
2021-05-01: 64K+
2021-05-02: 61K+
2021-05-03: 57K+
2021-05-04: 45K+
2021-05-05: 32K+
2021-05-06: 31K+
2021-05-07: 25K+
2021-05-08: 15K+
...
It is therefore not clear to me why the problems should only have started on May 6th. I haven't received any problem reports on this issue either.
Reporter | ||
Comment 4•3 years ago
|
||
Out of 183 reported crashes for Firefox 88.0.1 and 88.0 (based on 100+ installations), 162 have tampermonkey installed and 155 uBlock. The issue isn't bound to the latest versions of the add-ons because a comparison of two crashes found the following version:
- firefox%40tampermonkey.net: 4.12.6132, 4.13.6136
- uBlock0%40raymondhill.net: 1.35.0, 1.35.2
First crash report for Firefox 88.0 submitted at 2021-05-06 22:52:22 UTC - might be triggered by an external change of web content.
Assignee | ||
Comment 5•3 years ago
|
||
It seems plausible for the doc to be unlinked, or for the element to be moved
to another document, in between the time observer notification is scheduled and
the time it runs. In that case, we'd be unregistered already anyways, so
there's nothing else to do.
Updated•3 years ago
|
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7a990505a788 Null-check lazyload intersection observers on unregistration. r=sefeng
Assignee | ||
Comment 7•3 years ago
|
||
Comment on attachment 9221111 [details]
Bug 1710283 - Null-check lazyload intersection observers on unregistration. r=sefeng
Beta/Release Uplift Approval Request
- User impact if declined: Crashes
- Is this code covered by automated tests?: No
- Has the fix been verified in Nightly?: No
- Needs manual test from QE?: No
- If yes, steps to reproduce: (There are no known STR)
- List of other uplifts needed: none
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): Null-check
- String changes made/needed: none
Comment 8•3 years ago
|
||
Not only approval-mozilla-beta
, but also approval-mozilla-release
is needed I think?
Assignee | ||
Comment 9•3 years ago
|
||
Comment on attachment 9221111 [details]
Bug 1710283 - Null-check lazyload intersection observers on unregistration. r=sefeng
Beta/Release Uplift Approval Request
- User impact if declined: see above
- Is this code covered by automated tests?: No
- Has the fix been verified in Nightly?: No
- Needs manual test from QE?: No
- If yes, steps to reproduce: no concrete steps
- List of other uplifts needed: none
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): null-check
- String changes made/needed: none
Comment 10•3 years ago
|
||
bugherder |
Comment 11•3 years ago
|
||
Emilio, your patch does not graft cleanly to beta, could you rebase it please? Thanks
Assignee | ||
Comment 12•3 years ago
|
||
Comment 13•3 years ago
|
||
Comment on attachment 9221289 [details] [diff] [review]
Beta patch
Let's uplift to beta as the volume should tell us rapidly if this fixes the problem, thanks!
Updated•3 years ago
|
Comment 14•3 years ago
|
||
bugherder uplift |
Comment 15•3 years ago
|
||
The fix was effective and we have no crashes with this signature since 89 beta 11, thanks.
Comment 16•3 years ago
|
||
Comment on attachment 9221111 [details]
Bug 1710283 - Null-check lazyload intersection observers on unregistration. r=sefeng
Fx89 is in RC now, no need to take in 88.
Updated•3 years ago
|
Description
•