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

Windows nightly fails on open #17290

Closed
jonathandturner opened this issue Jun 13, 2017 · 25 comments
Closed

Windows nightly fails on open #17290

jonathandturner opened this issue Jun 13, 2017 · 25 comments

Comments

@jonathandturner
Copy link

@jonathandturner jonathandturner commented Jun 13, 2017

Running the current Windows nightly, whether .msi or .zip install, fails to run. The servo screen closes immediately on open.

Nothing is printed to the terminal when this occurs.

@NiLSPACE
Copy link

@NiLSPACE NiLSPACE commented Jun 13, 2017

I get this error:

called `Result::unwrap()` on an `Err` value: RelativeUrlWithoutBase (thread main, at src\libcore\result.rs:860)
@jdm
Copy link
Member

@jdm jdm commented Jun 13, 2017

This sounds like a regression from #16477.

@jdm
Copy link
Member

@jdm jdm commented Jun 13, 2017

@sadmansk
Copy link
Contributor

@sadmansk sadmansk commented Jun 13, 2017

I will get a windows virtual machine up and running and check what this is all about

@jonathandturner
Copy link
Author

@jonathandturner jonathandturner commented Jun 13, 2017

Does anyone know if we can get one of the test machines to smoketest running the Windows build? That way hopefully we could catch the catastrophic stuff before it lands?

@jdm
Copy link
Member

@jdm jdm commented Jun 13, 2017

The tricky bit would be controlling the browser through automation. We would want to be sure that the browser.html interface has finished loading, and then quit the browser, then check the process return code.

@jonathandturner
Copy link
Author

@jonathandturner jonathandturner commented Jun 13, 2017

@jdm - yeah for something more complicated I can see that being harder, though for cases like "browser fails immediately" hopefully we can catch them. If we could start them up and screenshot them and compare to a known working screenshot... that'd be even better (but I'll take what I can 😄 )

@jdm
Copy link
Member

@jdm jdm commented Jun 13, 2017

Right, I'm just pointing out that even the most basic test of "did the browser.html interface load successfully" probably requires using something like webdriver.

@sadmansk
Copy link
Contributor

@sadmansk sadmansk commented Jun 14, 2017

@jdm I'm currently running into something similar to this issue #12356, download of rustc-nightly-i686-pc-windows-msvc.tar.gz fails. I tried manually downloading and extracting x86_64 as per the instructions, it still tries to download the i686 version when I ran the build script. Any idea how to get around this issue?

@paulrouget
Copy link
Contributor

@paulrouget paulrouget commented Jun 14, 2017

@NiLSPACE can you please add the backtrace? (RUST_BACKTRACE=1)

@paulrouget
Copy link
Contributor

@paulrouget paulrouget commented Jun 14, 2017

@sadmansk Can you download the nightly build and reproduce the crash?

@sadmansk
Copy link
Contributor

@sadmansk sadmansk commented Jun 14, 2017

@paulrouget Yeap, it crashes for me in windows.

@sadmansk
Copy link
Contributor

@sadmansk sadmansk commented Jun 14, 2017

Issue is not isolated to windows, the nightly build for linux also crashes for me with the same error.

@sadmansk
Copy link
Contributor

@sadmansk sadmansk commented Jun 14, 2017

I'm having a hard time reproducing the error when building from source, but I think the culprit is this, in file components/config/opts.rs:647::

    let url_opt = url_opt.and_then(|url_string| parse_url_or_filename(&cwd, url_string)
                                   .or_else(|error| {
                                       warn!("URL parsing failed ({:?}).", error);
                                       Err(error)
                                   }).ok());

In case the parsing fails, it issues a warning and then tries to call ok() on Err(error). In that case, should I replace Err(error) with Ok(ServoUrl::parse("").unwrap()) or Ok(ServoUrl::parse("about:blank").unwrap())?

@paulrouget
Copy link
Contributor

@paulrouget paulrouget commented Jun 14, 2017

it issues a warning and then tries to call ok() on Err(error)

Which is fine. It will return None, which is what we expect.

@paulrouget
Copy link
Contributor

@paulrouget paulrouget commented Jun 14, 2017

@sadmansk:

on Linux, do this: export RUST_BACKTRACE=1. This will give you more details about the crash.

@sadmansk
Copy link
Contributor

@sadmansk sadmansk commented Jun 14, 2017

@paulrouget This is the output:

called `Result::unwrap()` on an `Err` value: RelativeUrlWithoutBase (thread main, at /checkout/src/libcore/result.rs:860)
stack backtrace:
   0:     0x5613ae6806fc - backtrace::backtrace::trace::h58bdbc4722aadbf4
   1:     0x5613ae680842 - backtrace::capture::Backtrace::new::hde1afc496271e12d
   2:     0x5613acaa4e1d - servo::main::{{closure}}::h1637c169f0f2e7e5
   3:     0x5613aea8ce2a - std::panicking::rust_panic_with_hook
                        at /checkout/src/libstd/panicking.rs:550
   4:     0x5613aea8ccc4 - std::panicking::begin_panic<collections::string::String>
                        at /checkout/src/libstd/panicking.rs:511
   5:     0x5613aea8cbf9 - std::panicking::begin_panic_fmt
                        at /checkout/src/libstd/panicking.rs:495
   6:     0x5613aea8cb87 - std::panicking::rust_begin_panic
                        at /checkout/src/libstd/panicking.rs:471
   7:     0x5613aeac55dd - core::panicking::panic_fmt
                        at /checkout/src/libcore/panicking.rs:69
   8:     0x5613aca8f1a5 - core::result::unwrap_failed::h8508ab846c072e92
   9:     0x5613acaa3b17 - servo::main::h8fa98aecb40c4fa8
  10:     0x5613aea93efa - panic_unwind::__rust_maybe_catch_panic
                        at /checkout/src/libpanic_unwind/lib.rs:98
  11:     0x5613aea8d568 - std::panicking::try<(),closure>
                        at /checkout/src/libstd/panicking.rs:433
                         - std::panic::catch_unwind<closure,()>
                        at /checkout/src/libstd/panic.rs:361
                         - std::rt::lang_start
                        at /checkout/src/libstd/rt.rs:59
  12:     0x7fd65acc8439 - __libc_start_main
  13:     0x5613aca6dc29 - <unknown>
  14:                0x0 - <unknown>
@joshumax
Copy link

@joshumax joshumax commented Jun 14, 2017

Here's my backtrace -- looks the same but I haven't diffed it with the one above:

called `Result::unwrap()` on an `Err` value: RelativeUrlWithoutBase (thread main, at /checkout/src/libcore/result.rs:860)
stack backtrace:
   0:     0x5617be12b6fc - backtrace::backtrace::trace::h58bdbc4722aadbf4
   1:     0x5617be12b842 - backtrace::capture::Backtrace::new::hde1afc496271e12d
   2:     0x5617bc54fe1d - servo::main::{{closure}}::h1637c169f0f2e7e5
   3:     0x5617be537e2a - std::panicking::rust_panic_with_hook
                        at /checkout/src/libstd/panicking.rs:550
   4:     0x5617be537cc4 - std::panicking::begin_panic<collections::string::String>
                        at /checkout/src/libstd/panicking.rs:511
   5:     0x5617be537bf9 - std::panicking::begin_panic_fmt
                        at /checkout/src/libstd/panicking.rs:495
   6:     0x5617be537b87 - std::panicking::rust_begin_panic
                        at /checkout/src/libstd/panicking.rs:471
   7:     0x5617be5705dd - core::panicking::panic_fmt
                        at /checkout/src/libcore/panicking.rs:69
   8:     0x5617bc53a1a5 - core::result::unwrap_failed::h8508ab846c072e92
   9:     0x5617bc54eb17 - servo::main::h8fa98aecb40c4fa8
  10:     0x5617be53eefa - panic_unwind::__rust_maybe_catch_panic
                        at /checkout/src/libpanic_unwind/lib.rs:98
  11:     0x5617be538568 - std::panicking::try<(),closure>
                        at /checkout/src/libstd/panicking.rs:433
                         - std::panic::catch_unwind<closure,()>
                        at /checkout/src/libstd/panic.rs:361
                         - std::rt::lang_start
                        at /checkout/src/libstd/rt.rs:59
  12:     0x7f486d6f62b0 - __libc_start_main
  13:     0x5617bc518c29 - <unknown>
  14:                0x0 - <unknown>

Can confirm this is happening on a Debian 9 machine too

@paulrouget
Copy link
Contributor

@paulrouget paulrouget commented Jun 14, 2017

In main.rs:

    let target_url = opts::get().url.clone()
                .unwrap_or(ServoUrl::parse(PREFS.get("shell.homepage").as_string()
                    .unwrap_or("about:blank")).unwrap());

PREFS.get("shell.homepage").as_string() might return something, but not a valid a url?

@paulrouget
Copy link
Contributor

@paulrouget paulrouget commented Jun 14, 2017

"shell.homepage" is a local file. It needs to go through parse_url_or_filename to become a valid url.

That's why it's crashing.

@sadmansk
Copy link
Contributor

@sadmansk sadmansk commented Jun 14, 2017

@paulrouget just so it's clear, it should become:

                .unwrap_or(parse_url_or_filename(PREFS.get("shell.homepage").as_string()
                    .unwrap_or("about:blank")).unwrap());

?

@paulrouget
Copy link
Contributor

@paulrouget paulrouget commented Jun 14, 2017

I think this should do it:

    let target_url = opts::get().url.clone().or(
         PREFS.get("shell.homepage").as_string().map(|str| parse_url_or_filename(str))
    ).or(ServoUrl::parse("about:blank")).unwrap();

I haven't tried.

But the unwrapping soup is confusing. Maybe you can simplify this with more steps:

     let cwd = env::current_dir().unwrap();
     let cmdline_url = opts::get().url.clone();
     let pref_url = PREFS.get("shell.homepage").as_string()
         .and_then(|str| parse_url_or_filename(&cwd, str).ok());
     let blank_url = ServoUrl::parse("about:blank").ok();
     let target_url = cmdline_url.or(pref_url).or(blank_url).unwrap();
@paulrouget
Copy link
Contributor

@paulrouget paulrouget commented Jun 14, 2017

(I've updated the code in my latest comment)

@sadmansk sadmansk mentioned this issue Jun 14, 2017
3 of 5 tasks complete
@soapdog
Copy link

@soapdog soapdog commented Jun 16, 2017

Setting RUST_BACKTRACE to 1 on Windows 10 doesn't give me a backtrace as can be seen on the shot below. Is there another way? Is a backtrace from Win 10 wanted?

select windows powershell 2017-06-16 01 58 14

@jdm
Copy link
Member

@jdm jdm commented Jun 16, 2017

It should not be necessary; we have backtrace in a previous comment already.

bors-servo added a commit that referenced this issue Jun 17, 2017
Fix parsing of shell.homepage

<!-- Please describe your changes on the following line: -->
This should fix the nightly builds crashing, although I don't know how to verify whether my fixes worked since I don't know how to test packaging. Any pointers on how to do that would be highly appreciated.

@paulrouget Please let me know if there are any problems with these changes.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #17290
<!-- 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. -->

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

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