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

crash in ScriptThread loading vimeo player #26930

Closed
philip-lamb opened this issue Jun 15, 2020 · 4 comments
Closed

crash in ScriptThread loading vimeo player #26930

philip-lamb opened this issue Jun 15, 2020 · 4 comments

Comments

@philip-lamb
Copy link

@philip-lamb philip-lamb commented Jun 15, 2020

Config:

Servo CI build macOS 0.0.1-d43f8ee792
macOS 10.15.5 (19F101)

/Volumes/Servo/Servo.app/Contents/MacOS/servo https://vimeo.com/channels/staffpicks/429098251
No valid entry type provided to observe().
%c 
⎜       @@@&       @@@@@@
⎜    #@@@@@@@    &@@@@@@@@       Built using:
⎜  #@@@@@@@@@.  %%&@@@@@@@        • React + Flux
⎜      &@@@@@%     (@@@@@(       • ES6
⎜       @@@@@@     @@@@@@        • Fetch API
⎜       %@@@@@(  .@@@@@(         • Flexbox
⎜        @@@@@@(@@@@@@
⎜        /@@@@@@@@@@%            Like playing with new technology, too?
⎜         &@@@@@@@%              https://vimeo.com/jobs
⎜          @@@@@#


color: #00adef;
assertion failed: !obj.is_null() (thread ScriptThread PipelineId { namespace_id: PipelineNamespaceId(1), index: PipelineIndex(1) }, at components/script/dom/globalscope.rs:2089)
[2020-06-15T23:17:19Z ERROR servo] assertion failed: !obj.is_null()
assertion failed: !cx.is_null() (thread ScriptThread PipelineId { namespace_id: PipelineNamespaceId(1), index: PipelineIndex(1) }, at /Users/worker/.cargo/git/checkouts/rust-mozjs-8611526964119dd6/28248e1/src/rust.rs:311)
[2020-06-15T23:17:19Z ERROR servo] assertion failed: !cx.is_null()
thread panicked while panicking. aborting.
[2020-06-15T23:17:19Z ERROR constellation::constellation] about:failure failed
Stack trace for thread "ScriptThread PipelineId { namespace_id: PipelineNamespaceId(1), index: PipelineIndex(1) }"
   0: <servo::backtrace::Print as core::fmt::Debug>::fmt
   1: core::fmt::write
   2: std::io::Write::write_fmt
   3: servo::backtrace::print
   4: servo::install_crash_handler::handler
   5: __sigtramp
   6: std::panicking::rust_panic_with_hook
   7: std::panicking::begin_panic
   8: mozjs::rust::Runtime::get
   9: core::ptr::drop_in_place
  10: core::ptr::drop_in_place
  11: <alloc::rc::Rc<T> as core::ops::drop::Drop>::drop
  12: core::ptr::drop_in_place
  13: std::sys_common::backtrace::__rust_begin_short_backtrace
  14: core::ops::function::FnOnce::call_once{{vtable.shim}}
  15: std::sys::unix::thread::Thread::new::thread_start
  16: __pthread_start

@jdm
Copy link
Member

@jdm jdm commented Jun 16, 2020

* thread #50, stop reason = breakpoint 1.1
    frame #0: 0x0000000107273874 servo`rust_panic at panicking.rs:529:9 [opt]
    frame #1: 0x000000010727375d servo`std::panicking::rust_panic_with_hook::hf8b9378dd2e7986a at panicking.rs:499:5 [opt]
    frame #2: 0x0000000107311981 servo`std::panicking::begin_panic::hd82eda01e9352594(msg=(data_ptr = "assertion failed: !obj.is_null()assertion failed: !global.is_null()PageErrorscriptexpected a Window scope\x01evaluating Dom stringerror evaluating Dom stringassertion failed: !cx.is_null()measurenavigationpaint", length = 32)) at panicking.rs:404:5
    frame #3: 0x000000010118479e servo`script::dom::globalscope::GlobalScope::from_object::h6de5b9f25e075ff2(obj=0x0000000000000000) at globalscope.rs:2089:9
    frame #4: 0x0000000101a8e106 servo`script::script_runtime::enqueue_promise_job::_$u7b$$u7b$closure$u7d$$u7d$::h2b4a499149082406 at script_runtime.rs:209:22
    frame #5: 0x0000000102b43313 servo`core::ops::function::impls::_$LT$impl$u20$core..ops..function..FnOnce$LT$A$GT$$u20$for$u20$$RF$mut$u20$F$GT$::call_once::h8623ea681e67a6ab(self=&mut FnMut<()> @ 0x0000700010ebdeb0, args=<unavailable>) at function.rs:285:13
    frame #6: 0x0000000102b47d55 servo`_$LT$std..panic..AssertUnwindSafe$LT$F$GT$$u20$as$u20$core..ops..function..FnOnce$LT$$LP$$RP$$GT$$GT$::call_once::h70fc4892823da655(self=(__0 = core::ops::function::&mut FnMut<()> @ 0x0000700010ebdee0), _args=<unavailable>) at panic.rs:318:9
    frame #7: 0x0000000102b3fc56 servo`std::panicking::try::do_call::h79c4b46d0f1b6e9a(data="8��") at panicking.rs:297:40
    frame #8: 0x0000000102b4022d servo`__rust_try + 29
    frame #9: 0x0000000102b3fbb1 servo`std::panicking::try::hf761b7e1892512d1(f=(__0 = core::ops::function::&mut FnMut<()> @ 0x0000700010ebdfd0)) at panicking.rs:274:15
    frame #10: 0x0000000102b47ec5 servo`std::panic::catch_unwind::h7345ac675f3cebec(f=(__0 = core::ops::function::&mut FnMut<()> @ 0x0000700010ebe040)) at panic.rs:394:14
    frame #11: 0x0000000102b3f655 servo`mozjs::panic::wrap_panic::h23e35389c33b1e46(function=&mut FnMut<()> @ 0x0000700010ebe0a0) at panic.rs:22:11
    frame #12: 0x0000000101a8e0a5 servo`script::script_runtime::enqueue_promise_job::h734a81107b52cb26(extra=0x00000001289e2010, cx=0x000000011a8ba000, promise=Handle<*mut mozjs_sys::generated::root::JSObject> @ 0x0000700010ebe0e8, job=Handle<*mut mozjs_sys::generated::root::JSObject> @ 0x0000700010ebe0f8, _allocation_site=Handle<*mut mozjs_sys::generated::root::JSObject> @ 0x0000700010ebe108, incumbent_global=Handle<*mut mozjs_sys::generated::root::JSObject> @ 0x0000700010ebe118) at script_runtime.rs:207:5
  * frame #13: 0x0000000102b4fced servo`RustJobQueue::enqueuePromiseJob(this=0x0000000115221230, cx=0x000000011a8ba000, promise=JS::HandleObject @ 0x0000700010ebe1d8, job=JS::HandleObject @ 0x0000700010ebe1d0, allocationSite=JS::HandleObject @ 0x0000700010ebe1c8, incumbentGlobal=JS::HandleObject @ 0x0000700010ebe1c0) at jsglue.cpp:62:12
    frame #14: 0x0000000102d47b59 servo`JSRuntime::enqueuePromiseJob(this=<unavailable>, cx=0x000000011a8ba000, job=JS::HandleFunction @ r15, promise=<unavailable>, incumbentGlobal=Handle<js::GlobalObject *> @ r14) at Runtime.cpp:633:24 [opt]
    frame #15: 0x0000000102ca92b9 servo`EnqueuePromiseReactionJob(cx=0x000000011a8ba000, reactionObj=<unavailable>, handlerArg_=<unavailable>, targetState=<unavailable>) at Promise.cpp:1214:25 [opt]
    frame #16: 0x0000000102c8f9fc servo`ResolvePromise(JSContext*, JS::Handle<js::PromiseObject*>, JS::Handle<JS::Value>, JS::PromiseState) at Promise.cpp:1541:10 [opt]
    frame #17: 0x0000000102c8f918 servo`ResolvePromise(cx=0x000000011a8ba000, promise=<unavailable>, valueOrReason=<unavailable>, state=Fulfilled) at Promise.cpp:1262 [opt]
    frame #18: 0x0000000102ca74af servo`FulfillMaybeWrappedPromise(cx=0x000000011a8ba000, promiseObj=<unavailable>, value_=<unavailable>) at Promise.cpp:1299:10 [opt]
    frame #19: 0x0000000102c8acc9 servo`ResolvePromiseInternal(cx=0x000000011a8ba000, promise=<unavailable>, resolutionVal=<unavailable>) at Promise.cpp:999:12 [opt]
    frame #20: 0x0000000102ca66f7 servo`ResolvePromiseFunction(cx=0x000000011a8ba000, argc=<unavailable>, vp=0x0000700010ebe678) at Promise.cpp:1066:8 [opt]
    frame #21: 0x000039ba29d9b42c
@jdm jdm added the C-reproduced label Jun 16, 2020
@jdm
Copy link
Member

@jdm jdm commented Jun 16, 2020

The incumbent_global argument we're getting for enqueue_promise_job is null, which is surprising. I'm going to make a debug mozjs build to try and figure out why that's happening.

@jdm
Copy link
Member

@jdm jdm commented Jun 16, 2020

This diff avoids the problem but I'm not yet convinced that it's the correct thing to do:

diff --git a/components/script/script_runtime.rs b/components/script/script_runtime.rs
index 4cf2055c64..a0a771941b 100644
--- a/components/script/script_runtime.rs
+++ b/components/script/script_runtime.rs
@@ -206,7 +206,12 @@ unsafe extern "C" fn enqueue_promise_job(
     let mut result = false;
     wrap_panic(&mut || {
         let microtask_queue = &*(extra as *const MicrotaskQueue);
-        let global = GlobalScope::from_object(incumbent_global.get());
+        let global = if !incumbent_global.is_null() {
+            GlobalScope::from_object(incumbent_global.get())
+        } else {
+            let realm = AlreadyInRealm::assert_for_cx(cx);
+            GlobalScope::from_context(*cx, InRealm::in_realm(&realm))
+        };
         let pipeline = global.pipeline_id();
         let interaction = if promise.get().is_null() {
             PromiseUserInputEventHandlingState::DontCare
@jdm
Copy link
Member

@jdm jdm commented Jun 16, 2020

We get into this situation from https://github.com/servo/mozjs/blob/aabcc9ba889b2755f1e4e83f28323a60415a790f/mozjs/js/src/builtin/Promise.cpp#L3025, so my patch looks reasonable to me.

@jdm jdm mentioned this issue Jun 16, 2020
4 of 4 tasks complete
@jdm jdm self-assigned this Jun 16, 2020
bors-servo added a commit that referenced this issue Jun 16, 2020
Fix panic on vimeo.com

The only time we end up enqueuing a promise reaction job with a null incumbent global argument is when SpiderMonkey does some weird stuff behind the scenes for its JS Debugger support (based on https://searchfox.org/mozilla-central/rev/2c1092dc68c63f7bad6da6a03c5883a5ab5ff2ca/js/src/builtin/Promise.cpp#3168). This commit works around that case and adds a regression test so we don't forget about that in the future.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #26930
- [x] There are tests for these changes
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.