Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Borrow failure if a major GC happens while running ScriptThread::handle_fetch_metadata #26857

Closed
jdm opened this issue Jun 10, 2020 · 1 comment
Closed

Comments

@jdm
Copy link
Member

@jdm jdm commented Jun 10, 2020

This happens because of this code which can end up allocating JS objects and triggering a GC. When we trace the script thread thread, we hit a borrow error when trying to trace the incomplete_parser_contexts field.

    frame #40: 0x00000001094d9bc5 servo`core::option::expect_none_failed::h4732b42c308e77b2 at option.rs:1272:5 [opt]
    frame #41: 0x0000000102c42256 servo`core::result::Result$LT$T$C$E$GT$::expect::hb58fa8778b031a24(self=Result<core::cell::Ref<alloc::vec::Vec<(msg::constellation_msg::PipelineId, script::dom::servoparser::ParserContext)>>, core::cell::BorrowError> @ 0x000070000549eec8, msg=(data_ptr = "already mutably borrowed/rustc/a74d1862d4d87a56244958416fd05976c58ca1a8/src/libcore/char/methods.rsencode_utf8: need  bytes to encode U+, but the buffer has to_digit: radix is too high (maximum 36)/rustc/a74d1862d4d87a56244958416fd05976c58ca1a8/src/libcore/iter/traits/iterator.rs/rustc/a74d1862d4d87a56244958416fd05976c58ca1a8/src/libcore/alloc/layout.rs/rustc/a74d1862d4d87a56244958416fd05976c58ca1a8/src/liballoc/slice.rs�StylesheetInvalidationSet::collect_invalidations_forstyle::invalidation::stylesheetscomponents/style/invalidation/stylesheets.rs > Fully invalid already > Stylesheet was not effective > resulting subtree invalidations:  > resulting self invalidations:  > fully_invalid: /Users/jdm/.cargo/registry/src/github.com-1ecc6299db9ec823/chrono-0.4.9/src/naive/date.rs", length = 24)) at result.rs:963:23
    frame #42: 0x0000000103d84207 servo`core::cell::RefCell$LT$T$GT$::borrow::heb01ab788059a28e(self=0x00007000054ad088) at cell.rs:798:9
    frame #43: 0x0000000103dde559 servo`_$LT$core..cell..RefCell$LT$T$GT$$u20$as$u20$script..dom..bindings..trace..JSTraceable$GT$::trace::h9954e2c8d284eede(self=0x00007000054ad088, trc=0x00007fa89d82cfb8) at trace.rs:305:9
    frame #44: 0x0000000102e34918 servo`_$LT$script..script_thread..ScriptThread$u20$as$u20$script..dom..bindings..trace..JSTraceable$GT$::trace::h9c7305ebaccb1bd6(self=0x00007000054acfe8, tracer=0x00007fa89d82cfb8) at script_thread.rs:509:10
    frame #45: 0x0000000102d29883 servo`script::script_thread::trace_thread::_$u7b$$u7b$closure$u7d$$u7d$::h5f6ee50e13a1786d(root=0x00007fa89d822400) at script_thread.rs:172:13
    frame #46: 0x000000010375c0f6 servo`std::thread::local::LocalKey$LT$T$GT$::try_with::h2457fa24c2d097a1(self=0x0000000109ee0b60, f=closure-0 @ 0x000070000549f510) at local.rs:263:16
    frame #47: 0x0000000103753f95 servo`std::thread::local::LocalKey$LT$T$GT$::with::h865c45833990cd27(self=0x0000000109ee0b60, f=closure-0 @ 0x000070000549f578) at local.rs:239:9
    frame #48: 0x0000000102d29754 servo`script::script_thread::trace_thread::hf643e803a42d5c95(tr=0x00007fa89d82cfb8) at script_thread.rs:169:5
    frame #49: 0x0000000103c634b5 servo`script::script_runtime::trace_rust_roots::h1d068df55861be1f(tr=0x00007fa89d82cfb8, _data=0x0000000000000000) at script_runtime.rs:818:5
    frame #50: 0x0000000105150982 servo`js::gc::GCRuntime::traceRuntimeCommon(JSTracer*, js::gc::GCRuntime::TraceOrMarkRuntime) at RootMarking.cpp:444:5 [opt]
    frame #51: 0x0000000105150954 servo`js::gc::GCRuntime::traceRuntimeCommon(this=<unavailable>, trc=<unavailable>, traceOrMark=MarkRuntime) at RootMarking.cpp:429 [opt]
    frame #52: 0x00000001051502c9 servo`js::gc::GCRuntime::traceRuntimeForMajorGC(this=0x00007fa89d82c320, trc=0x00007fa89d82cfb8, session=<unavailable>) at RootMarking.cpp:290:3 [opt]
    frame #53: 0x0000000105128dcf servo`js::gc::GCRuntime::beginMarkPhase(this=0x00007fa89d82c320, reason=<unavailable>, session=0x000070000549f940) at GC.cpp:4060:5 [opt]
    frame #54: 0x00000001051310af servo`js::gc::GCRuntime::incrementalSlice(this=0x00007fa89d82c320, budget=<unavailable>, gckind=0x000070000549f9c8, reason=TOO_MUCH_MALLOC, session=0x000070000549f940) at GC.cpp:6758:12 [opt]
    frame #55: 0x000000010513245c servo`js::gc::GCRuntime::gcCycle(this=0x00007fa89d82c320, nonincrementalByAPI=true, budget=SliceBudget @ 0x000070000549f9a0, gckind=<unavailable>, reason=TOO_MUCH_MALLOC) at GC.cpp:7243:3 [opt]
    frame #56: 0x0000000105132d0b servo`js::gc::GCRuntime::collect(this=0x00007fa89d82c320, nonincrementalByAPI=true, budget=SliceBudget @ 0x000070000549fa10, gckindArg=<unavailable>, reason=TOO_MUCH_MALLOC) at GC.cpp:7428:9 [opt]
    frame #57: 0x0000000105124f54 servo`js::gc::GCRuntime::startGC(this=0x00007fa89d82c320, gckind=GC_NORMAL, reason=TOO_MUCH_MALLOC, millis=<unavailable>) at GC.cpp:0 [opt]
    frame #58: 0x0000000105117080 servo`js::gc::GCRuntime::gcIfNeededAtAllocation(JSContext*) [inlined] js::gc::GCRuntime::gcIfRequested(this=0x00007fa89d82c320) at GC.cpp:7712:7 [opt]
    frame #59: 0x0000000105117077 servo`js::gc::GCRuntime::gcIfNeededAtAllocation(this=0x00007fa89d82c320, cx=0x00007fa89d828800) at Allocator.cpp:369 [opt]
    frame #60: 0x0000000105115dd1 servo`js::BaseShape* js::Allocate<js::BaseShape, (js::AllowGC)1>(JSContext*) [inlined] bool js::gc::GCRuntime::checkAllocatorState<(js::AllowGC)1>(this=<unavailable>, cx=<unavailable>, kind=<unavailable>) at Allocator.cpp:326:10 [opt]
    frame #61: 0x0000000105115dc9 servo`js::BaseShape* js::Allocate<js::BaseShape, (js::AllowGC)1>(cx=0x00007fa89d828800) at Allocator.cpp:251 [opt]
    frame #62: 0x0000000104f2e1cc servo`js::BaseShape::getUnowned(cx=0x00007fa89d828800, base=0x000070000549fbe8) at Shape.cpp:1618:23 [opt]
    frame #63: 0x0000000104f327e3 servo`js::EmptyShape::getInitialShape(cx=0x00007fa89d828800, clasp=0x0000000109fcc650, proto=TaggedProto @ 0x000070000549fd10, nfixed=0, objectFlags=0) at Shape.cpp:2168:39 [opt]
    frame #64: 0x0000000104ec077e servo`NewObject(cx=0x00007fa89d828800, group=<unavailable>, kind=<unavailable>, newKind=SingletonObject, initialShapeFlags=<unavailable>) at JSObject.cpp:787:25 [opt]
    frame #65: 0x0000000104ec0324 servo`js::NewObjectWithGivenTaggedProto(cx=0x00007fa89d828800, clasp=0x0000000109fcc650, proto=<unavailable>, allocKind=<unavailable>, newKind=<unavailable>, initialShapeFlags=0) at JSObject.cpp:867:20 [opt]
    frame #66: 0x0000000104de28f9 servo`JS_NewObjectWithUniqueType(JSContext*, JSClass const*, JS::Handle<JSObject*>) [inlined] js::NewObjectWithGivenTaggedProto(cx=<unavailable>, clasp=<unavailable>, proto=<unavailable>, newKind=SingletonObject, initialShapeFlags=0) at JSObject-inl.h:418:10 [opt]
    frame #67: 0x0000000104de28ae servo`JS_NewObjectWithUniqueType(JSContext*, JSClass const*, JS::Handle<JSObject*>) [inlined] js::NewObjectWithGivenProto(cx=<unavailable>, clasp=<unavailable>, proto=<unavailable>, newKind=SingletonObject) at JSObject-inl.h:462 [opt]
    frame #68: 0x0000000104de28ae servo`JS_NewObjectWithUniqueType(cx=<unavailable>, clasp=<unavailable>, proto=JS::HandleObject @ r14) at jsfriendapi.cpp:129 [opt]
    frame #69: 0x000000010346ae81 servo`mozjs::rust::wrappers::JS_NewObjectWithUniqueType::hf0bcc963d11a0467(cx=0x00007fa89d828800, clasp=0x0000000109fcc650, proto=Handle<*mut mozjs_sys::generated::root::JSObject> @ 0x000070000549fea0) at rust.rs:1453:51
    frame #70: 0x00000001034f7e5b servo`script::dom::bindings::interface::create_object::h8b5f325f38db7db1(cx=JSContext @ 0x000070000549ff60, global=Handle<*mut mozjs_sys::generated::root::JSObject> @ 0x000070000549ff68, proto=Handle<*mut mozjs_sys::generated::root::JSObject> @ 0x000070000549ff70, class=0x0000000109fcc650, methods=&[script::dom::bindings::guard::Guard<&[mozjs_sys::generated::root::JSFunctionSpec]>] @ 0x000070000549ff80, properties=&[script::dom::bindings::guard::Guard<&[mozjs_sys::generated::root::JSPropertySpec]>] @ 0x000070000549ff90, constants=&[script::dom::bindings::guard::Guard<&[script::dom::bindings::constant::ConstantSpec]>] @ 0x000070000549ffa0, rval=MutableHandle<*mut mozjs_sys::generated::root::JSObject> @ 0x000070000549fff0) at interface.rs:319:18
    frame #71: 0x00000001034f70bc servo`script::dom::bindings::interface::create_interface_prototype_object::hea8f1884e6ae183e(cx=JSContext @ 0x00007000054a00a8, global=Handle<*mut mozjs_sys::generated::root::JSObject> @ 0x00007000054a0100, proto=Handle<*mut mozjs_sys::generated::root::JSObject> @ 0x00007000054a0108, class=0x0000000109fcc650, regular_methods=&[script::dom::bindings::guard::Guard<&[mozjs_sys::generated::root::JSFunctionSpec]>] @ 0x00007000054a0118, regular_properties=&[script::dom::bindings::guard::Guard<&[mozjs_sys::generated::root::JSPropertySpec]>] @ 0x00007000054a0128, constants=&[script::dom::bindings::guard::Guard<&[script::dom::bindings::constant::ConstantSpec]>] @ 0x00007000054a0138, unscopable_names=&[&[u8]] @ 0x00007000054a0148, rval=MutableHandle<*mut mozjs_sys::generated::root::JSObject> @ 0x00007000054a01e0) at interface.rs:195:5
    frame #72: 0x000000010467cdb5 servo`script::dom::bindings::codegen::Bindings::PerformanceEntryBinding::PerformanceEntryBinding::CreateInterfaceObjects::h778cb91b322c69c2(cx=JSContext @ 0x00007000054a03b8, global=Handle<*mut mozjs_sys::generated::root::JSObject> @ 0x00007000054a0440, cache=0x0000000157bd9000) at PerformanceEntryBinding.rs:1103:5
    frame #73: 0x000000010467c7af servo`script::dom::bindings::codegen::Bindings::PerformanceEntryBinding::PerformanceEntryBinding::GetProtoObject::h5651c8d682e3939f(cx=JSContext @ 0x00007000054a04d0, global=Handle<*mut mozjs_sys::generated::root::JSObject> @ 0x00007000054a04b8, rval=MutableHandle<*mut mozjs_sys::generated::root::JSObject> @ 0x00007000054a04c0) at PerformanceEntryBinding.rs:1029:9
    frame #74: 0x0000000103e53796 servo`script::dom::bindings::codegen::Bindings::PerformanceResourceTimingBinding::PerformanceResourceTimingBinding::CreateInterfaceObjects::h1227d3766d7bac52(cx=JSContext @ 0x00007000054a06f8, global=Handle<*mut mozjs_sys::generated::root::JSObject> @ 0x00007000054a0780, cache=0x0000000157bd9000) at PerformanceResourceTimingBinding.rs:1944:5
    frame #75: 0x0000000103e5337f servo`script::dom::bindings::codegen::Bindings::PerformanceResourceTimingBinding::PerformanceResourceTimingBinding::GetProtoObject::h1dd3996793b9aec5(cx=JSContext @ 0x00007000054a0810, global=Handle<*mut mozjs_sys::generated::root::JSObject> @ 0x00007000054a07f8, rval=MutableHandle<*mut mozjs_sys::generated::root::JSObject> @ 0x00007000054a0800) at PerformanceResourceTimingBinding.rs:1874:9
    frame #76: 0x0000000103563a06 servo`script::dom::bindings::codegen::Bindings::PerformanceNavigationTimingBinding::PerformanceNavigationTimingBinding::CreateInterfaceObjects::hb3cdfa7dc69a75e6(cx=JSContext @ 0x00007000054a0a18, global=Handle<*mut mozjs_sys::generated::root::JSObject> @ 0x00007000054a0aa0, cache=0x0000000157bd9000) at PerformanceNavigationTimingBinding.rs:1641:5
    frame #77: 0x000000010356374f servo`script::dom::bindings::codegen::Bindings::PerformanceNavigationTimingBinding::PerformanceNavigationTimingBinding::GetProtoObject::hd7de42dd85342cee(cx=JSContext @ 0x00007000054a0b30, global=Handle<*mut mozjs_sys::generated::root::JSObject> @ 0x00007000054a0b18, rval=MutableHandle<*mut mozjs_sys::generated::root::JSObject> @ 0x00007000054a0b20) at PerformanceNavigationTimingBinding.rs:1591:9
    frame #78: 0x00000001035630c3 servo`script::dom::bindings::codegen::Bindings::PerformanceNavigationTimingBinding::PerformanceNavigationTimingBinding::Wrap::h3a7219526c30a5bf(cx=JSContext @ 0x00007000054a0cf0, scope=0x000000013f613000, object=0x0000000157b40f80) at PerformanceNavigationTimingBinding.rs:1333:5
    frame #79: 0x00000001036138a1 servo`script::dom::bindings::reflector::reflect_dom_object::ha0e2e39783652e7f(obj=0x0000000157b40f80, global=0x000000013f613000) at reflector.rs:25:14
    frame #80: 0x0000000103369cd4 servo`script::dom::performancenavigationtiming::PerformanceNavigationTiming::new::h1aa39667244427ee(global=0x000000013f613000, nav_start=0, nav_start_precise=0, document=0x000000013f613a00) at performancenavigationtiming.rs:55:9
    frame #81: 0x00000001037f6678 servo`_$LT$script..dom..servoparser..ParserContext$u20$as$u20$net_traits..FetchResponseListener$GT$::submit_resource_timing::h1d4903023f67a80e(self=0x00000001249500a8) at mod.rs:939:13
    frame #82: 0x00000001037f5091 servo`_$LT$script..dom..servoparser..ParserContext$u20$as$u20$net_traits..FetchResponseListener$GT$::process_response::h5e23229ea7fe056a(self=0x00000001249500a8, meta_result=Result<net_traits::FetchMetadata, net_traits::NetworkError> @ 0x00007000054a3360) at mod.rs:792:9
  * frame #83: 0x0000000102d544d2 servo`script::script_thread::ScriptThread::handle_fetch_metadata::h140ae982cce6f862(self=0x00007000054acfe8, id=PipelineId @ 0x00007000054a32b8, fetch_metadata=Result<net_traits::FetchMetadata, net_traits::NetworkError> @ 0x00007000054a3fc8) at script_thread.rs:3720:13
    frame #84: 0x0000000102d3a0f0 servo`script::script_thread::ScriptThread::handle_msg_from_constellation::h9cb8adac23f3022a(self=0x00007000054acfe8, msg=ConstellationControlMsg @ 0x00007000054a5618) at script_thread.rs:1813:25
    frame #85: 0x0000000102d3544b servo`script::script_thread::ScriptThread::handle_msgs::_$u7b$$u7b$closure$u7d$$u7d$::h85cf3fe1cc175208 at script_thread.rs:1539:53
    frame #86: 0x0000000102d38680 servo`script::script_thread::ScriptThread::profile_event::h7140b5fbdfcaf31e(self=0x00007000054acfe8, category=ConstellationMsg, pipeline_id=Option<msg::constellation_msg::PipelineId> @ 0x00007000054a5bd8, f=closure-6 @ 0x00007000054ac4a8) at script_thread.rs:1784:13
    frame #87: 0x0000000102d339dc servo`script::script_thread::ScriptThread::handle_msgs::h811bc866555a3cb6(self=0x00007000054acfe8) at script_thread.rs:1533:26
    frame #88: 0x0000000102d313d0 servo`script::script_thread::ScriptThread::start::h48e822917d4ce601(self=0x00007000054acfe8) at script_thread.rs:1356:15
    frame #89: 0x0000000102d2b30b servo`_$LT$script..script_thread..ScriptThread$u20$as$u20$script_traits..ScriptThreadFactory$GT$::create::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::h8bf7a94bcecac64b at script_thread.rs:796:25
    frame #90: 0x000000010309eb18 servo`profile_traits::mem::ProfilerChan::run_with_memory_reporting::h8577d8126fbc77d3(self=0x00007000054acfcc, f=closure-1 @ 0x00007000054ace50, reporter_name=String @ 0x00007000054ad9c8, channel_for_reporter=(flavor = crossbeam_channel::channel::SenderFlavor<script::script_thread::MainThreadScriptMsg> @ 0x00007000054acd80), msg=(0x0000000000000015)) at mem.rs:88:9
    frame #91: 0x0000000102d2ba33 servo`_$LT$script..script_thread..ScriptThread$u20$as$u20$script_traits..ScriptThreadFactory$GT$::create::_$u7b$$u7b$closure$u7d$$u7d$::h58160dcac2a3794c at script_thread.rs:794:17
    frame #92: 0x0000000103ae6653 servo`std::sys_common::backtrace::__rust_begin_short_backtrace::h2352de0ff94aa7f6(f=<unavailable>) at backtrace.rs:130:5
    frame #93: 0x0000000102eb71f3 servo`std::thread::Builder::spawn_unchecked::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::h0a490641a735c3ec at mod.rs:475:17
    frame #94: 0x000000010486f513 servo`_$LT$std..panic..AssertUnwindSafe$LT$F$GT$$u20$as$u20$core..ops..function..FnOnce$LT$$LP$$RP$$GT$$GT$::call_once::h7a1b073cd6f775a2(self=<unavailable>, _args=<unavailable>) at panic.rs:318:9
    frame #95: 0x0000000102bdb578 servo`std::panicking::try::do_call::haadabca21c978f5f(data="") at panicking.rs:297:40
    frame #96: 0x0000000102e36c3d servo`__rust_try + 29
    frame #97: 0x0000000102bdb327 servo`std::panicking::try::hca2bc7b8227c229e(f=<unavailable>) at panicking.rs:274:15
    frame #98: 0x00000001048899c3 servo`std::panic::catch_unwind::h1a0acd87547dfad8(f=<unavailable>) at panic.rs:394:14
    frame #99: 0x0000000102eb6305 servo`std::thread::Builder::spawn_unchecked::_$u7b$$u7b$closure$u7d$$u7d$::h4cd1ce5e1ebeb5a0 at mod.rs:474:30
    frame #100: 0x0000000103f21661 servo`core::ops::function::FnOnce::call_once$u7b$$u7b$vtable.shim$u7d$$u7d$::hcf0da69f624e806f((null)=0x00000001248e4000, (null)=<unavailable>) at function.rs:232:5
    frame #101: 0x000000010942befd servo`std::sys::unix::thread::Thread::new::thread_start::h2abb2e9c73153285 [inlined] _$LT$alloc..boxed..Box$LT$F$GT$$u20$as$u20$core..ops..function..FnOnce$LT$A$GT$$GT$::call_once::hb81a25054e2a996f at boxed.rs:1034:9 [opt]
    frame #102: 0x000000010942bef7 servo`std::sys::unix::thread::Thread::new::thread_start::h2abb2e9c73153285 [inlined] _$LT$alloc..boxed..Box$LT$F$GT$$u20$as$u20$core..ops..function..FnOnce$LT$A$GT$$GT$::call_once::h47c4f8d42821f188 at boxed.rs:1034 [opt]
    frame #103: 0x000000010942beee servo`std::sys::unix::thread::Thread::new::thread_start::h2abb2e9c73153285 at thread.rs:87 [opt]
    frame #104: 0x00007fff70094109 libsystem_pthread.dylib`_pthread_start + 148
    frame #105: 0x00007fff7008fb8b libsystem_pthread.dylib`thread_start + 15
@gterzian
Copy link
Member

@gterzian gterzian commented Jun 11, 2020

I think probably the "right" way to fix this is to use ParserContext via an IPC route, see for example

struct BroadcastListener {
and how it's used in a route at
ROUTER.add_route(

(actually, the ParserContext already contains a Trusted<ServoParser>, and all other fields are not DOM managed, so it's pretty much ready to use in an IPC route).


The "quick" solution would be, I think, to scope the borrow and clone the ParserContext, before calling one of the process_ methods on it.

Also, this would have to be done for each handle_fetch_ method on ScriptThread.

Actually, just cloning is not enough, since you need to carry state across various calls, so probably would need to take it out, drop the borrow, call the method, and then re-borrow and put it back in the vector...

Maybe it would be cleaner to do it properly by restructuring the flow to use the ROUTER.

@gterzian gterzian mentioned this issue Jun 17, 2020
0 of 5 tasks complete
bors-servo added a commit that referenced this issue Jun 18, 2020
Do not trace pending parsers

<!-- Please describe your changes on the following line: -->

FIX #26857

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Jun 18, 2020
Do not trace pending parsers

<!-- Please describe your changes on the following line: -->

FIX #26857

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants
You can’t perform that action at this time.