Crash in [@ nsNavHistoryResult::HandlePlacesEvent]
Categories
(Toolkit :: Places, defect)
Tracking
()
People
(Reporter: gerard-majax, Assigned: daisuke)
References
(Blocks 1 open bug)
Details
(Keywords: crash, Whiteboard: [sng])
Crash Data
Attachments
(1 file)
Crash report: https://crash-stats.mozilla.org/report/index/470c3890-5796-4007-aaa2-8bf800240110
MOZ_CRASH Reason: MOZ_DIAGNOSTIC_ASSERT(false) (NS_SUCCEEDED(NS_NewURI(getter_AddRefs(uri), item->mUrl)))
Top 10 frames of crashing thread:
0 libxul.so nsNavHistoryResult::HandlePlacesEvent toolkit/components/places/nsNavHistoryResult.cpp:4243
1 libxul.so std::function<void const /builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/include/c++/8/bits/std_function.h:687
1 libxul.so mozilla::dom::CallListeners<mozilla::WeakPtr<mozilla::places::INativePlacesEventCallback, dom/base/PlacesObservers.cpp:112
1 libxul.so mozilla::dom::PlacesObservers::NotifyNext dom/base/PlacesObservers.cpp:350
2 libxul.so mozilla::dom::PlacesObservers::NotifyListeners dom/base/PlacesObservers.cpp:298
2 libxul.so mozilla::dom::PlacesObservers_Binding::notifyListeners dom/bindings/PlacesObserversBinding.cpp:402
3 libxul.so CallJSNative js/src/vm/Interpreter.cpp:479
3 libxul.so js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:573
3 libxul.so InternalCall js/src/vm/Interpreter.cpp:640
3 libxul.so js::CallFromStack js/src/vm/Interpreter.cpp:645
My nightly suddenly decided not to start anymore:
- try to launch
- browser shows
- wait
- ends up crashing before you can interact
I've disabled a few things, reset the places db to no luck.
somehow the only fix was user_pref("identity.fxaccounts.enabled", false);. Probably a broken entry there?
|
Reporter | |
Comment 1•2 years ago
|
||
I ran my profile against a debug build and I got that:
(gdb) f 8
#8 nsNavHistoryResult::HandlePlacesEvent (this=0x7f353782b940, aEvents=...) at /home/alex/codaz/Mozilla/gecko-cinnabar/toolkit/components/places/nsNavHistoryResult.cpp:4243
4243 MOZ_ALWAYS_SUCCEEDS(NS_NewURI(getter_AddRefs(uri), item->mUrl));
(gdb) l
4238 continue;
4239 }
4240
4241 nsCOMPtr<nsIURI> uri;
4242 if (item->mItemType == nsINavBookmarksService::TYPE_BOOKMARK) {
4243 MOZ_ALWAYS_SUCCEEDS(NS_NewURI(getter_AddRefs(uri), item->mUrl));
4244 if (!uri) {
4245 continue;
4246 }
4247 }
(gdb) print item->mUrl
$1 = {<nsTSubstring<char16_t>> = {<mozilla::detail::nsTStringRepr<char16_t>> = {static kMaxCapacity = 1073741822, mData = 0x7f353440dc08 u"https://www.42registry.42/", mLength = {static kMax = 1073741822, mLength = 26}, mDataFlags = 5, mClassFlags = mozilla::detail::StringClassFlags::NULL_TERMINATED}, <No data fields>}, <No data fields>}
(gdb) bt
#0 0x00007f35774e56f1 in __GI___clock_nanosleep (clock_id=clock_id@entry=0, flags=flags@entry=0, req=req@entry=0x7ffd7b706370, rem=rem@entry=0x7ffd7b706370) at ../sysdeps/unix/sysv/linux/clock_nanosleep.c:78
#1 0x00007f35774f9937 in __GI___nanosleep (req=req@entry=0x7ffd7b706370, rem=rem@entry=0x7ffd7b706370) at ../sysdeps/unix/sysv/linux/nanosleep.c:25
#2 0x00007f357750e2ae in __sleep (seconds=0) at ../sysdeps/posix/sleep.c:55
#3 0x00007f356e440240 in common_crap_handler (signum=11, aFirstFramePC=<optimized out>) at /home/alex/codaz/Mozilla/gecko-cinnabar/toolkit/xre/nsSigHandlers.cpp:100
#4 0x00007f356e44032d in ah_crap_handler (signum=0) at /home/alex/codaz/Mozilla/gecko-cinnabar/toolkit/xre/nsSigHandlers.cpp:108
#5 0x00007f356e414331 in nsProfileLock::FatalSignalHandler (signo=11, info=0x7ffd7b706670, context=0x7ffd7b706540) at /home/alex/codaz/Mozilla/gecko-cinnabar/toolkit/profile/nsProfileLock.cpp:183
#6 0x00007f356fa15219 in WasmTrapHandler (signum=11, info=0x7ffd7b706670, context=0x7ffd7b706540) at /home/alex/codaz/Mozilla/gecko-cinnabar/js/src/wasm/WasmSignalHandlers.cpp:794
#7 0x00007f3577442910 in <signal handler called> () at /lib/x86_64-linux-gnu/libc.so.6
#8 nsNavHistoryResult::HandlePlacesEvent (this=0x7f353782b940, aEvents=...) at /home/alex/codaz/Mozilla/gecko-cinnabar/toolkit/components/places/nsNavHistoryResult.cpp:4243
#9 0x00007f356927bd19 in std::function<void (RefPtr<mozilla::places::INativePlacesEventCallback>&, mozilla::dom::Sequence<mozilla::OwningNonNull<mozilla::dom::PlacesEvent> > const&)>::operator()(RefPtr<mozilla::places::INativePlacesEventCallback>&, mozilla::dom::Sequence<mozilla::OwningNonNull<mozilla::dom::PlacesEvent> > const&) const (this=0x7ffd7b707408, __args=..., __args=...)
at /home/alex/.mozbuild/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/std_function.h:687
#10 mozilla::dom::CallListeners<mozilla::WeakPtr<mozilla::places::INativePlacesEventCallback, (mozilla::detail::WeakPtrDestructorBehavior)0>, RefPtr<mozilla::places::INativePlacesEventCallback>, mozilla::dom::ListenerCollection<mozilla::WeakPtr<mozilla::places::INativePlacesEventCallback, (mozilla::detail::WeakPtrDestructorBehavior)0> > >(unsigned int, mozilla::dom::Sequence<mozilla::OwningNonNull<mozilla::dom::PlacesEvent> > const&, unsigned long, std::function<RefPtr<mozilla::places::INativePlacesEventCallback> (mozilla::WeakPtr<mozilla::places::INativePlacesEventCallback, (mozilla::detail::WeakPtrDestructorBehavior)0>&)> const&, std::function<void (RefPtr<mozilla::places::INativePlacesEventCallback>&, mozilla::dom::Sequence<mozilla::OwningNonNull<mozilla::dom::PlacesEvent> > const&)> const&)
(aEventFlags=aEventFlags@entry=14, aEvents=..., aListenersLengthToCall=aListenersLengthToCall@entry=8, aUnwrapListener=..., aCallListener=...) at /home/alex/codaz/Mozilla/gecko-cinnabar/dom/base/PlacesObservers.cpp:112
#11 0x00007f35692717ab in mozilla::dom::PlacesObservers::NotifyNext () at /home/alex/codaz/Mozilla/gecko-cinnabar/dom/base/PlacesObservers.cpp:350
#12 0x00007f35692715a0 in mozilla::dom::PlacesObservers::NotifyListeners (aEvents=...) at /home/alex/codaz/Mozilla/gecko-cinnabar/dom/base/PlacesObservers.cpp:324
#13 0x00007f35698b58ce in mozilla::dom::PlacesObservers_Binding::notifyListeners (cx_=cx_@entry=0x7f355f333c00, argc=<optimized out>, vp=<optimized out>) at ./PlacesObserversBinding.cpp:402
#14 0x00007f356e66ba95 in CallJSNative (cx=cx@entry=0x7f355f333c00, native=native@entry=0x7f35698b5360 <mozilla::dom::PlacesObservers_Binding::notifyListeners(JSContext*, unsigned int, JS::Value*)>, reason=reason@entry=js::CallReason::Call, args=...) at /home/alex/codaz/Mozilla/gecko-cinnabar/js/src/vm/Interpreter.cpp:479
#15 0x00007f356e644129 in js::InternalCallOrConstruct (cx=0x7f355f333c00, args=..., construct=construct@entry=js::NO_CONSTRUCT, reason=js::CallReason::Call) at /home/alex/codaz/Mozilla/gecko-cinnabar/js/src/vm/Interpreter.cpp:573
#16 0x00007f356e64508d in InternalCall (cx=0x7f35776008e0 <_IO_stdfile_2_lock>, args=..., reason=140439376) at /home/alex/codaz/Mozilla/gecko-cinnabar/js/src/vm/Interpreter.cpp:640
#17 0x00007f356e655b04 in js::CallFromStack (cx=0x7f35776008e0 <_IO_stdfile_2_lock>, args=..., reason=<optimized out>) at /home/alex/codaz/Mozilla/gecko-cinnabar/js/src/vm/Interpreter.cpp:645
#18 js::Interpret (cx=0x7f355f333c00, state=...) at /home/alex/codaz/Mozilla/gecko-cinnabar/js/src/vm/Interpreter.cpp:3060
#19 0x00007f356e643a09 in MaybeEnterInterpreterTrampoline (cx=0x7f35776008e0 <_IO_stdfile_2_lock>, cx@entry=0x7f355f333c00, state=...) at /home/alex/codaz/Mozilla/gecko-cinnabar/js/src/vm/Interpreter.cpp:393
#20 0x00007f356e6436bf in js::RunScript (cx=cx@entry=0x7f355f333c00, state=...) at /home/alex/codaz/Mozilla/gecko-cinnabar/js/src/vm/Interpreter.cpp:451
#21 0x00007f356e644049 in js::InternalCallOrConstruct (cx=0x7f355f333c00, args=..., construct=construct@entry=js::NO_CONSTRUCT, reason=js::CallReason::Call) at /home/alex/codaz/Mozilla/gecko-cinnabar/js/src/vm/Interpreter.cpp:605
#22 0x00007f356e64508d in InternalCall (cx=0x7f35776008e0 <_IO_stdfile_2_lock>, cx@entry=0x7f355f333c00, args=..., reason=140439376, reason@entry=js::CallReason::Call) at /home/alex/codaz/Mozilla/gecko-cinnabar/js/src/vm/Interpreter.cpp:640
#23 0x00007f356e6452ae in js::Call (cx=cx@entry=0x7f355f333c00, fval=fval@entry=..., thisv=thisv@entry=..., args=..., rval=rval@entry=..., reason=reason@entry=js::CallReason::Call) at /home/alex/codaz/Mozilla/gecko-cinnabar/js/src/vm/Interpreter.cpp:672
#24 0x00007f356ea0e498 in js::CallSelfHostedFunction (cx=cx@entry=0x7f355f333c00, name=..., thisv=..., args=..., rval=rval@entry=...) at /home/alex/codaz/Mozilla/gecko-cinnabar/js/src/vm/SelfHosting.cpp:1520
#25 0x00007f356f268512 in js::jit::InterpretResume (cx=0x7f355f333c00, obj=..., stackValues=<optimized out>, rval=...) at /home/alex/codaz/Mozilla/gecko-cinnabar/js/src/jit/VMFunctions.cpp:1075
#26 0x000023229cef604a in ??? ()
#27 0xfff9800000000000 in ??? ()
#28 0x000000000000009e in ??? ()
#29 0x00007ffd7b707f80 in ??? ()
#30 0x000023229cf770e0 in ??? ()
#31 0x0000000000000001 in ??? ()
#32 0x00003650d93912f0 in ??? ()
#33 0x00007ffd7b707f18 in ??? ()
#34 0xfff8800000000000 in ??? ()
#35 0xfff9800000000000 in ??? ()
#36 0xfffe3650d93912f0 in ??? ()
#37 0xfffe3650d93912f0 in ??? ()
#38 0x0000000000000000 in ??? ()
(gdb)
The URL mentionned is https://www.42registry.42/ and over all attempts it was always crashing on this one. Not sure if it's just luck or that this entry has something ...
|
|
Reporter | |
Comment 2•2 years ago
|
||
21:29:00,068 uri = Services.io.newURI("https://www.google.com");
21:29:00,097
XPCWrappedNative_NoHelper { spec: Getter, prePath: Getter, scheme: Getter, userPass: Getter, username: Getter, password: Getter, hostPort: Getter, host: Getter, port: Getter, pathQueryRef: Getter, … }
21:29:02,782 uri = Services.io.newURI("https://www.42registry.42/");
21:29:02,800
Uncaught NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIIOService.newURI]
<anonymous> debugger eval code:1
getEvalResult resource://devtools/server/actors/webconsole/eval-with-debugger.js:300
evalWithDebugger resource://devtools/server/actors/webconsole/eval-with-debugger.js:212
evaluateJS resource://devtools/server/actors/webconsole.js:948
evaluateJSAsync resource://devtools/server/actors/webconsole.js:846
makeInfallible resource://devtools/shared/ThreadSafeDevToolsUtils.js:103
debugger eval code:1:19
|
|
Reporter | |
Comment 3•2 years ago
|
||
FTR, this reproduces even with nightly from a few weeks ago. Something might have changed on Sync/FxAccount side recently that exposed it.
|
|
Reporter | |
Comment 4•2 years ago
|
||
Ok, so:
- I in fact found a matching bookmark with that url
- removed it
- re-enabled Sync/FxA
- it's not crashing anymore
I suspect we might not want to crash so badly ?
Daisuke you were the last one hacking around that, do you have an opinion? Just NS_SUCCEED() instead of MOZ_ALWAYS_SUCCEED() maybe?
|
Assignee | |
Comment 5•2 years ago
|
||
Hi :gerard-majax, thank you very much for the report!
As it might be better to check the URL before nsNavHistoryResult::HandlePlacesEvent(), I will take a look at this.
|
||
Comment 6•2 years ago
•
|
||
All the places APIs check for uri validity, it looks like there's some Sync code not doing that?
Edit: oh, I see the uri parser was changed, sigh :( Ok maybe we don't want a diagnostic assert around newURI, but just warnings and bailouts, as this can arguably happen if uri parsing changes.
https://searchfox.org/mozilla-central/search?q=MOZ_ALWAYS_SUCCEEDS(NS_&path=places&case=false®exp=false
|
|
Reporter | |
Comment 7•2 years ago
|
||
I have kept the offending profile, so we can investigate if you want
|
|
Reporter | |
Comment 8•2 years ago
|
||
hm the broken profile I kept sync'd again with FxA and ... The bookmark disappeared :[
|
|
Reporter | |
Comment 9•2 years ago
|
||
(In reply to Marco Bonardo [:mak] from comment #6)
All the places APIs check for uri validity, it looks like there's some Sync code not doing that?
Edit: oh, I see the uri parser was changed, sigh :( Ok maybe we don't want a diagnostic assert around newURI, but just warnings and bailouts, as this can arguably happen if uri parsing changes.
https://searchfox.org/mozilla-central/search?q=MOZ_ALWAYS_SUCCEEDS(NS_&path=places&case=false®exp=false
Are you referring to bug 1723456 ? I was about to throw a patch changing MOZ_ALWAYS_SUCCEEDS into NS_SUCCEEDED checks, but nika mentionned there might be legit cases of doing the assert?
|
|
Reporter | |
Comment 10•2 years ago
|
||
I still have some matches, if it can help:
$ rg "42registry" yakxclvy.default*/*
yakxclvy.default/lock: No such file or directory (os error 2)
yakxclvy.default/places.sqlite.backup: binary file matches (found "\0" byte around offset 15)
yakxclvy.default/placescleaner_places.bak: binary file matches (found "\0" byte around offset 15)
yakxclvy.default/places.sqlite: binary file matches (found "\0" byte around offset 15)
yakxclvy.default_FAILED/places.sqlite.backup: binary file matches (found "\0" byte around offset 15)
yakxclvy.default_FAILED/placescleaner_places.bak: binary file matches (found "\0" byte around offset 15)
yakxclvy.default_FAILED/places.sqlite: binary file matches (found "\0" byte around offset 15)
yakxclvy.default_FAILED/cert8.db.old: binary file matches (found "\0" byte around offset 0)
yakxclvy.default_DEBUG/places.sqlite.backup: binary file matches (found "\0" byte around offset 15)
yakxclvy.default_DEBUG/placescleaner_places.bak: binary file matches (found "\0" byte around offset 15)
yakxclvy.default_DEBUG/places.sqlite: binary file matches (found "\0" byte around offset 15)
yakxclvy.default_DEBUG/cert8.db.old: binary file matches (found "\0" byte around offset 0)
yakxclvy.default/cert8.db.old: binary file matches (found "\0" byte around offset 0)
yakxclvy.default_backup/places.sqlite-wal: binary file matches (found "\0" byte around offset 4)
yakxclvy.default_backup/places.sqlite.backup: binary file matches (found "\0" byte around offset 15)
yakxclvy.default_backup/placescleaner_places.bak: binary file matches (found "\0" byte around offset 15)
yakxclvy.default_backup/places.sqlite: binary file matches (found "\0" byte around offset 15)
yakxclvy.default_backup/cert8.db.old: binary file matches (found "\0" byte around offset 0)
|
|
Reporter | |
Comment 11•2 years ago
|
||
Exploring places.sqlite, filtering on 42registry:
- table
moz_originsreturns one hit, - table
moz_placesreturns one hit,
Is it possible that the trigger point is rather Sync trying to read the data out of Places?
|
|
Reporter | |
Comment 12•2 years ago
|
||
|
|
||
Comment 13•2 years ago
|
||
the trigger point is that we assert, Sync doesn't matter, if the url is inserted before the parser change, later calls trying to build a uri out of those entries will fail.
Yes, the assert may be useful to detect this, but it's unnecessarily annoying for users, as we see it's easy to crash with simple parser changes.
I'd rather change to warnings, and fix bug 1470043 so periodically we remove invalid uris.
|
|
Reporter | |
Comment 14•2 years ago
|
||
(In reply to Marco Bonardo [:mak] from comment #13)
the trigger point is that we assert, Sync doesn't matter, if the url is inserted before the parser change, later calls trying to build a uri out of those entries will fail.
Yes, the assert may be useful to detect this, but it's unnecessarily annoying for users, as we see it's easy to crash with simple parser changes.
I'd rather change to warnings, and fix bug 1470043 so periodically we remove invalid uris.
Let me know if you need my places database
|
||
Comment 15•2 years ago
|
||
(In reply to Marco Bonardo [:mak] from comment #6)
All the places APIs check for uri validity, it looks like there's some Sync code not doing that?
FWIW, bug 1874233 adds a test to verify we do the right thing for incoming invalid URLs.
Edit: oh, I see the uri parser was changed, sigh
welp, yeah, that seems like it's a problem, but I'm still not sure what happened here. IIUC, our rust-based mobile clients would never have considered that URL valid. I'm not even sure from the above whether sync is still implicated here.
|
|
Assignee | |
Updated•2 years ago
|
|
|
Assignee | |
Comment 16•2 years ago
|
||
Okay, I will change to warning instead.
|
|
Assignee | |
Comment 17•2 years ago
|
||
|
||
Updated•2 years ago
|
|
|
||
Updated•2 years ago
|
|
||
Comment 18•1 year ago
|
||
|
||
Comment 19•1 year ago
|
||
| bugherder | ||
|
||
Updated•1 year ago
|
|
||
Updated•1 year ago
|
|
||
Updated•1 year ago
|
Description
•