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

Webdriver panics when there isn't a root page #11117

Closed
asajeffrey opened this issue May 10, 2016 · 4 comments
Closed

Webdriver panics when there isn't a root page #11117

asajeffrey opened this issue May 10, 2016 · 4 comments
Assignees

Comments

@asajeffrey
Copy link
Member

@asajeffrey asajeffrey commented May 10, 2016

Running webdriver test https://github.com/w3c/web-platform-tests/blob/master/webdriver/navigation.py#L115 on Servo causes panic:

 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "stack backtrace:"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "ERROR:compositing::constellation: Panic: called `Option::unwrap()` on a `None` value"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "ERROR:compositing::constellation: Backtrace:"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #0  - 0x000055e6bb6d9db8 - backtrace::backtrace::libunwind::trace"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "                               @ /home/ajeffrey/github/asajeffrey/servo/.cargo/registry/src/github.com-88ac128001ac3a9a/backtrace-0.2.1/src/backtrace/libunwind.rs:54"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "                               - backtrace::backtrace::trace<closure>"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "                               @ /home/ajeffrey/github/asajeffrey/servo/.cargo/registry/src/github.com-88ac128001ac3a9a/backtrace-0.2.1/src/backtrace/mod.rs:70"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #1  - 0x000055e6bb6d9c6a - backtrace::capture::{{impl}}::new"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "                               @ /home/ajeffrey/github/asajeffrey/servo/target/debug/build/backtrace-bb3ee46349d51739/out/capture.rs:79"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #2  - 0x000055e6ba1b927c - util::thread::spawn_named_with_send_on_panic::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::hfb85bd26ec8873e5"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #3  - 0x000055e6ba1bb8ed - extern $u22$rust.call$u22$$u20$fn$LP$$u5b$closure$SP$DefId$u20$$u7b$$u20$krate.$u20$45$C$$u20$node.$u20$DefIndex$LP$2975$RP$$u20$$u7d$$u20$0.ipc_channel..ipc..IpcSender$LT$$LP$core..option..Option$LT$msg..constellation_msg..PipelineId$GT$$C$$u20$std..string..String$C$$u20$std..string..String$RP$$GT$$C$$u20$1.core..option..Option$LT$msg..constellation_msg..PipelineId$GT$$u5d$$C$$u20$$LP$$RF$$u27$static$u20$core..any..Any$u20$$u2b$$u20$$u27$static$C$$RP$$RP$::once_shim.164427::h3fbae899f360275b"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #4  - 0x000055e6ba1bb89c - alloc::boxed::{{impl}}::call_box<(&Any),closure>"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "                               @ ../src/liballoc/boxed.rs:541"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #5  - 0x000055e6bb6662dc - util::panicking::initiate_panic_hook::{{closure}}::{{closure}}::{{closure}}"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "                               @ /home/ajeffrey/github/asajeffrey/servo/components/util/panicking.rs:44"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #6  - 0x000055e6bb665e89 - std::thread::local::{{impl}}::with<core::cell::RefCell<core::option::Option<util::panicking::PanicHandlerLocal>>,closure,()>"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "                               @ ../src/libstd/thread/local.rs:211"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #7  - 0x000055e6bb665d0a - util::panicking::initiate_panic_hook::{{closure}}::{{closure}}"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "                               @ /home/ajeffrey/github/asajeffrey/servo/components/util/panicking.rs:41"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #8  - 0x000055e6bc154553 - std::sys_common::unwind::begin_unwind_inner::h30e12d15ce2b2e25"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #9  - 0x000055e6bc155f98 - std::sys_common::unwind::begin_unwind_fmt::hb2de8a9968d38523"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #10 - 0x000055e6bc1662f1 - rust_begin_unwind"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #11 - 0x000055e6bc1a589f - core::panicking::panic_fmt::h257ceb0aa351d801"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #12 - 0x000055e6bc1a5b78 - core::panicking::panic::h4bb1497076d04ab9"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #13 - 0x000055e6ba224bb2 - core::option::{{impl}}::unwrap<&alloc::rc::Rc<script::page::Page>>"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "                               @ ../src/libcore/macros.rs:21"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #14 - 0x000055e6ba0bae60 - script::script_thread::{{impl}}::root_page"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "                               @ /home/ajeffrey/github/asajeffrey/servo/components/script/script_thread.rs:597"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #15 - 0x000055e6ba25a09e - script::script_thread::{{impl}}::handle_webdriver_msg"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "                               @ /home/ajeffrey/github/asajeffrey/servo/components/script/script_thread.rs:995"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #16 - 0x000055e6ba2564ed - script::script_thread::{{impl}}::handle_msg_from_constellation"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "                               @ /home/ajeffrey/github/asajeffrey/servo/components/script/script_thread.rs:901"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #17 - 0x000055e6ba254b15 - script::script_thread::{{impl}}::handle_msgs::{{closure}}"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "                               @ /home/ajeffrey/github/asajeffrey/servo/components/script/script_thread.rs:766"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #18 - 0x000055e6ba254760 - script::script_thread::{{impl}}::profile_event<closure,core::option::Option<bool>>"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "                               @ /home/ajeffrey/github/asajeffrey/servo/components/script/script_thread.rs:858"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #19 - 0x000055e6ba227078 - script::script_thread::{{impl}}::handle_msgs"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "                               @ /home/ajeffrey/github/asajeffrey/servo/components/script/script_thread.rs:759"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #20 - 0x000055e6ba1cec5c - script::script_thread::{{impl}}::start"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "                               @ /home/ajeffrey/github/asajeffrey/servo/components/script/script_thread.rs:617"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #21 - 0x000055e6ba1ceb30 - script::script_thread::{{impl}}::create::{{closure}}::{{closure}}"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "                               @ /home/ajeffrey/github/asajeffrey/servo/components/script/script_thread.rs:469"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #22 - 0x000055e6ba1ce821 - profile_traits::mem::{{impl}}::run_with_memory_reporting<closure,fn(profile_traits::mem::ReportsChan) -> script::script_runtime::CommonScriptMsg,script::script_runtime::CommonScriptMsg,std::sync::mpsc::Sender<script::script_thread::MainThreadScriptMsg>>"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "                               @ /home/ajeffrey/github/asajeffrey/servo/components/profile_traits/mem.rs:59"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #23 - 0x000055e6ba1bc059 - script::script_thread::{{impl}}::create::{{closure}}"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "                               @ /home/ajeffrey/github/asajeffrey/servo/components/script/script_thread.rs:468"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #24 - 0x000055e6ba1b8bb6 - util::thread::spawn_named_with_send_on_panic::_$u7b$$u7b$closure$u7d$$u7d$::h153e74df3e754d9f"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #25 - 0x000055e6ba1b83f3 - std::thread::Builder::spawn::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::hdf1a857f1b00da32"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #26 - 0x000055e6ba1b8368 - std::sys_common::unwind::try::try_fn<closure>"
 0:04.18 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "                               @ ../src/libstd/sys/common/unwind/mod.rs:127"
 0:04.19 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #27 - 0x000055e6bc16627b - __rust_try"
 0:04.19 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #28 - 0x000055e6bc16620d - std::sys_common::unwind::inner_try::h47a4d9cd4a369dcd"
 0:04.19 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #29 - 0x000055e6ba1b82a5 - std::sys_common::unwind::try<closure>"
 0:04.19 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "                               @ ../src/libstd/sys/common/unwind/mod.rs:123"
 0:04.19 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #30 - 0x000055e6ba1b80e1 - std::thread::Builder::spawn::_$u7b$$u7b$closure$u7d$$u7d$::h4f3ea0e65664ee46"
 0:04.19 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #31 - 0x000055e6ba1b8da7 - alloc::boxed::{{impl}}::call_box<(),closure>"
 0:04.19 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "                               @ ../src/liballoc/boxed.rs:541"
 0:04.19 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #32 - 0x000055e6bc16ccd4 - std::sys::thread::Thread::new::thread_start::h6f266e069bf4ec2b"
 0:04.19 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #33 - 0x00007f99892bb6a9 - start_thread"
 0:04.19 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #34 - 0x00007f9988dd9e9c - clone"
 0:04.19 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "frame #35 - 0x0000000000000000 - <unknown>"
 0:04.19 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) ""
 0:04.19 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:6697) "ERROR:compositing::constellation: Pipeline failed in hard-fail mode.  Crashing!"

The root cause is https://github.com/servo/servo/blob/master/components/script/script_thread.rs#L995, which calls self.root_page() when a webdriver message arrives, generating a panic if there isn't a root page yet. (Navigation can cause a fresh script thread to be created, with root page initialized at None.)

cc @jgraham

@asajeffrey
Copy link
Member Author

@asajeffrey asajeffrey commented May 11, 2016

OK, diagnosis...

When script runs window.location.href = 'blah', this calls Location::SetHref:

    fn SetHref(&self, value: USVString) {
        if let Ok(url) = self.window.get_url().join(&value.0) {
            self.window.load_url(url);
        }
    }

which in turn calls Window::load_url:

   pub fn load_url(&self, url: Url) {
        let doc = self.Document();
        self.main_thread_script_chan().send(
            MainThreadScriptMsg::Navigate(self.id,
                LoadData::new(url, doc.get_referrer_policy(), Some(doc.url().clone())))).unwrap();
    }

this sends a MainThreadScriptMsg::Navigate message which calls ScriptThread::handle_navigate:

    fn handle_navigate(&self, pipeline_id: PipelineId, subpage_id: Option<SubpageId>, load_data: LoadData) {
        ...
        match subpage_id {
            Some(subpage_id) => ...
            None => {
                let ConstellationChan(ref const_chan) = self.constellation_chan;
                const_chan.send(ConstellationMsg::LoadUrl(pipeline_id, load_data)).unwrap();
            }
        }
    }

and sends a ConstellationMsg::LoadUrl which calls Constellation::load_url which creates the pipeline, but does not wait for it to finish loading, so its page is still None.

Now, when a webdriver command arrives, it gets routed to the current pipeline by webdriver::Handler::root_pipeline:

    fn root_pipeline(&self) -> WebDriverResult<PipelineId> {
        let interval = 20;
        let iterations = 30_000 / interval;

        for _ in 0..iterations {
            if let Some(x) = self.pipeline(None) {
                return Ok(x)
            };

            thread::sleep(Duration::from_millis(interval));
        };

        Err(WebDriverError::new(ErrorStatus::Timeout,
                                "Failed to get root window handle"))
    }

where webdriver::Handler::pipeline is:

    fn pipeline(&self, frame_id: Option<FrameId>) -> Option<PipelineId> {
        let (sender, receiver) = ipc::channel().unwrap();
        self.constellation_chan.send(ConstellationMsg::GetPipeline(frame_id, sender)).unwrap();


        receiver.recv().unwrap()
    }

and the constellation's handler for ConstellationMsg::GetPipeline is:

    fn handle_get_pipeline(&mut self, frame_id: Option<FrameId>,
                           resp_chan: IpcSender<Option<PipelineId>>) {
        let current_pipeline_id = frame_id.or(self.root_frame_id)
            .and_then(|frame_id| self.frames.get(&frame_id))
            .map(|frame| frame.current);
        let pipeline_id = self.pending_frames.iter().rev()
            .find(|x| x.old_pipeline_id == current_pipeline_id)
            .map(|x| x.new_pipeline_id).or(current_pipeline_id);
        if let Err(e) = resp_chan.send(pipeline_id) {
            warn!("Failed get_pipeline response ({}).", e);
        }
    }

so there is nothing waiting for the pipeline's page to finish loading before executing the command. In particular, this might be a script command, which gets routed to a script thread without a root page, and boom.

@asajeffrey
Copy link
Member Author

@asajeffrey asajeffrey commented May 11, 2016

@asajeffrey
Copy link
Member Author

@asajeffrey asajeffrey commented May 11, 2016

@asajeffrey
Copy link
Member Author

@asajeffrey asajeffrey commented May 11, 2016

Summarizing the IRC conversation... the only way we can get a pipeline without a page is for handle_get_pipeline to find a pending frame whose document_ready is false. We can fix this by returning document_ready back to webdriver, and having it wait for true.

bors-servo added a commit that referenced this issue May 12, 2016
…t-ready, r=jgraham

When webdriver is getting a pipeline id, it should wait for the pipeline document to be ready

Thank you for contributing to Servo! Please add an `X` inside each `[ ]` when the step is complete, and replace `__` with appropriate data:
- [X] `./mach build` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #11117 (github issue number if applicable).

Either:
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because they only impact webdriver

Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11140)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue May 12, 2016
…t-ready, r=jgraham

When webdriver is getting a pipeline id, it should wait for the pipeline document to be ready

Thank you for contributing to Servo! Please add an `X` inside each `[ ]` when the step is complete, and replace `__` with appropriate data:
- [X] `./mach build` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #11117 (github issue number if applicable).

Either:
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because they only impact webdriver

Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11140)
<!-- Reviewable:end -->
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.

1 participant
You can’t perform that action at this time.