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

Unwind error if the Coroutine::yield_with is inlined #51

Open
zonyitoo opened this issue Apr 23, 2016 · 5 comments
Open

Unwind error if the Coroutine::yield_with is inlined #51

zonyitoo opened this issue Apr 23, 2016 · 5 comments
Labels

Comments

@zonyitoo
Copy link
Owner

Now we can only add #[inline(never)] on the Coroutine::yield_with method.

You can reproduce it anytime by replacing it with #[inline(always)], and then call cargo test --release, you will see the coroutine_unwinds_on_drop test will fail.

$ cargo test --release
...
test coroutine::test::coroutine_unwinds_on_drop ... FAILED
...

failures:

---- coroutine::test::coroutine_unwinds_on_drop stdout ----
    thread 'coroutine::test::coroutine_unwinds_on_drop' panicked at 'assertion failed: `(left == right)` (left: `0`, right: `1`)', src/coroutine.rs:593
note: Run with `RUST_BACKTRACE=1` for a backtrace.


failures:
    coroutine::test::coroutine_unwinds_on_drop

test result: FAILED. 26 passed; 1 failed; 0 ignored; 0 measured

error: test failed
@zonyitoo zonyitoo added the bug label Apr 23, 2016
@lhecker
Copy link
Collaborator

lhecker commented Apr 24, 2016

We really should keep this issue open until we've found the reason.

It's not that bad that this method can't be inlined safely but it still annoys me to no end... The only thing that comes to my mind as a plausible reason though, is that Rust's (or LLVM's) ABI does something funky somewhere which Boost.Context does not know and thus something isn't carried over correctly. For instance some kind of stack frame pointer which isn't updated every time because LLVM viewed it as unnecessary.

@jClaireCodesStuff
Copy link

coroutine_unwind causes undefined behavior by unwinding through extern "C"

Proposed:

Check a flag after context.resume returns and start unwinding there.

Small performance hit, need to keep the flag in the Coroutine and check it on every switch, but it should allow inlining.

I'll try to get it working and PR later today.

@zonyitoo
Copy link
Owner Author

Hmm, I think it doesn't require a flag in Coroutine. You can make use of the data field in the Transport.

@jClaireCodesStuff
Copy link

Proof-of-concept in PR

I'll try to figure out passing through data.

@lhecker
Copy link
Collaborator

lhecker commented Apr 30, 2016

Hey @glasswings! If you intend to stick with this project for a while I can offer you to give you a quick introduction into how coio works and answer all your questions. You can ping me at my mail address if you want to and if you're comfortable with voice chat, because typing everything down would take ages. 😄

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

No branches or pull requests

3 participants