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

Update Hyper dependency #10

Closed
lhark opened this issue Feb 28, 2018 · 13 comments
Closed

Update Hyper dependency #10

lhark opened this issue Feb 28, 2018 · 13 comments

Comments

@lhark
Copy link

lhark commented Feb 28, 2018

The Hyper dependency is too old and prevents the build on up to date system as it itself depend on an old version of openssl-sys (0.74). More detail about the issue can be found here hyperium/hyper#985

I tried turning the version up to 0.11, but the compilation failed. However it succeeded with hyper = "0.10"

I'm not sure if it has anything to do with the other issue, but shadertoy-rs seems to crash while creating the windows in release mode. It works fine in debug mode. Here is the backtrace I got from the coredump:

[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib/libthread_db.so.1".
Core was generated by `./shadertoy --example seascape'.
Program terminated with signal SIGILL, Illegal instruction.
#0  0x000055d1ecc96506 in x11_dl::xlib_xcb::Xlib_xcb::open ()
(gdb) bt
#0  0x000055d1ecc96506 in x11_dl::xlib_xcb::Xlib_xcb::open ()
#1  0x000055d1ecc60590 in winit::api::x11::xdisplay::XConnection::new ()
#2  0x000055d1ecc76ccf in std::sync::once::Once::call_once::{{closure}} ()
#3  0x000055d1ecd1621d in std::sync::once::Once::call_inner ()
#4  0x000055d1ecc6303f in winit::os::unix::get_x11_xconnection ()
#5  0x000055d1ecc58a19 in glutin::platform::platform::x11::Window::new ()
#6  0x000055d1ecc5c6dd in glutin::window::<impl glutin::WindowBuilder<'a>>::build ()
#7  0x000055d1ecb71d35 in gfx_window_glutin::init_raw ()
#8  0x000055d1eca7014e in shadertoy::main ()
#9  0x000055d1eca6b3c3 in std::rt::lang_start::{{closure}} ()
#10 0x000055d1ecd1ac18 in std::panicking::try::do_call ()
#11 0x000055d1ecd2695f in __rust_maybe_catch_panic ()
#12 0x000055d1ecd1b54c in std::rt::lang_start_internal ()
#13 0x000055d1eca72ef4 in main ()

If you confirm the two issues aren't linked, I'll open another issue.

@fmenozzi
Copy link
Owner

fmenozzi commented Mar 1, 2018

What OS are you using? Rust version? Are your graphics drivers up-to-date?

On my machine (Ubuntu 16.04, Rust 1.22.1 stable) I don't have any issues with the current setup (i.e. hyper 0.7). If changing the dependency version to 0.10 works as a short-term solution, then I can do that, but I was also planning on migrating away from hyper to something more high-level like reqwests anyway, so hopefully this no longer becomes my issue.

@lhark
Copy link
Author

lhark commented Mar 1, 2018

I'm running Archlinux (kernel 4.15.5-1-ARCH), with rustc 1.24.0

Both my graphic drivers (integrated intel and GTX960m) are up to date:

nvidia 390.25-13
xf86-video-intel 1:2.99.917+812+g75795523-1

And there is no difference if I run shadertoy-rs on the intel GPU or the nvidia one: it runs perfectly in debug mode and crashes in release mode.

I'm not sure I fully understand the rust ecosystem yet, but I'm under the impression that -sys crates depend on the actual system library, in which case my version of openssl might be more recent: OpenSSL 1.1.0g 2 Nov 2017

@fmenozzi
Copy link
Owner

fmenozzi commented Mar 2, 2018

I'd be curious to see whether the rust version has anything to do with this. Do you have a way of reverting back to 1.22.1 stable and trying again (with both hyper 0.7 and hyper 0.10)? My internet connection here is quite unreliable so it might take me a while to upgrade to 1.24 and try it myself.

@lhark
Copy link
Author

lhark commented Mar 2, 2018

I was able to downgrade to rust 1.22.1, fortunately rust doesn't have any dependencies on my system. Here are the results with rust 1.22.1:

  • hyper 0.7
    • ❌ Still does not compile
  • hyper 0.10
    • Debug mode
      • ✅ Intel GPU
      • ✅ nvidia GPU
    • Release mode
      • ✅ Intel GPU
      • ✅ nvidia GPU

So while the downgrade didn't fix the problem with hyper compilation, it did fix the crash in release mode.

@fmenozzi
Copy link
Owner

fmenozzi commented Mar 2, 2018

So then it seems to me that there are two issues at play here: the later rust version that causes crashes in release mode, and the hyper version that causes compilation errors (likely to do with the system version of openssl).

Updating the hyper dependency would be a good short-term fix for the compilation error on older rust versions, but we're still left with the release crashes on the latest rust version. Maybe this warrants filing a bug with the rustc team? I normally try not to jump to the conclusion that there's a bug in the compiler but it's highly suspicious that this crate would work fine in 1.22 stable and crash with 1.24 stable. They also just released 1.24.1 stable because of regressions in other crates, so maybe it's not such a crazy idea after all.

In the meantime, I'll update the hyper dependency so people with older rust versions don't experience compile errors, but the crashing is going to require a deeper dive to figure out (ideally a bisect of the rustc code to find the commit that seemingly introduced the regression). I might also file a bug with the rustc folks to see if this might be something on their end.

A few more things we could try before going that far would be

  1. Upgrading to the current 1.24.1 patch release
  2. Updating other dependencies (since it's only crashing in release mode, it sounds like a memory bug, which makes me think that maybe a dependency that's using FFI/unsafe rust somehow got botched with this latest rust release)

@lhark
Copy link
Author

lhark commented Mar 2, 2018

Well, rust 1.24.1 didn't fix the problem :/

$ cargo run --release -- --example seascape
    Finished release [optimized] target(s) in 0.0 secs
     Running `target/release/shadertoy --example seascape`
zsh: illegal hardware instruction (core dumped)  cargo run --release -- --example seascape

I'm not sure sure my (very) limited rust experience will be enough to debug a compiler bug.
Which other dependency would you try to upgrade ? The backtrace seems to point towards a crash at window creation, so X11/OpenGL might be a good starting point.

@fmenozzi
Copy link
Owner

fmenozzi commented Mar 3, 2018

I agree, it looks like something during window creation, so I would try gfx, gfx_window_glutin, or glutin to see if upgrading those helps at all. If it doesn't, I can try and poke around on the subreddit to see if this problem might be a compiler bug of some kind and open an issue accordingly if it is.

@fmenozzi
Copy link
Owner

Sorry it's taken me so long to address this, I've been extremely busy.

I've added a new branch, update-dependencies, with an attempt at fixing this issue by updating the various graphics dependencies. Could you confirm that this new setup works for you on your machine? If so, I'll merge it in and close the issue.

@lhark
Copy link
Author

lhark commented Apr 29, 2018

No worries, I've also felt bad about doing more to help with this issue ^^

Good news! It runs perfectly both in debug and release mode, and on integrated and dedicated graphics.
Bad news, the keyboard event handling seems to have taken a hit in the process, Escape doesn't seem to work and even the WM shortcut refuses to work. The only way to quit is CTRL-C in the terminal.

I saw you reworked the event handling in runner.rs, seems like a prime suspect.

@fmenozzi
Copy link
Owner

So I was having a similar issue on my linux box (it does eventually close the window but it seems to take a minute or two). I figured this was an issue with my lack of graphics drivers, and my internet wasn't good enough to re-download/build the project on my windows box. This is partly why I asked you to confirm that it works for you. Since it doesn't, I'm not exactly sure what the issue could be, but I'll keep digging.

@lhark
Copy link
Author

lhark commented Apr 29, 2018

Now that you mention it, I retried and the keypresses do indeed register when you hold them down long enough (at least a few seconds).

@fmenozzi
Copy link
Owner

Yeah this is quite strange. I'm not entirely sure what's going on, so maybe I'll open an issue with the gfx project.

@fmenozzi
Copy link
Owner

Going to go ahead and close this given that your fix was just merged in. Thanks again for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants