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

Bug: Incompatibility of Rust's stdlib #56

Open
zonyitoo opened this Issue May 15, 2016 · 13 comments

Comments

Projects
None yet
6 participants
@zonyitoo
Copy link
Owner

zonyitoo commented May 15, 2016

This bug was originally found in #53 . The reason was: stdlib uses a PANIC_COUNT in TLS to ensure no panic while panicking in runtime. But obviously, Coroutines in coio can be migrated between threads, which means that it turns out to cause data race (because compiler still think that we are running in the same thread, so we may access to the other thread's TLS without any synchronization method).

We wanted to solve this PANIC_COUNT partially in rust-lang/rust#33408, but because stdlib relies heavily on TLS (such as println!), it will also causes SIGSEGV randomly:

println!("Before");
Scheduler::sched(); // Switch out
// Well, now, this Coroutine may already been stolen by the other thread
// And then resumed by the other thread
println!("After");

Rust's compiler don't know that we have switched to another thread, so it may inline those TLS calls.

We are looking for a solution for this bug, discussing in here, if you have any idea, please help.

@redbaron

This comment has been minimized.

Copy link
Contributor

redbaron commented Jul 19, 2016

how much it affects real-world applications? is there any way to check if given project is affected by imporper TLS handling?

@zonyitoo

This comment has been minimized.

Copy link
Owner Author

zonyitoo commented Jul 19, 2016

Unfortunately, I think every existing projects in Rust will be affected. Because nearly every Rust projects uses libstd, and libstd uses TLS for

  • Panic handling
  • Stdio

So, for example, if someone's project has println, it has already been affected.

println!("Print something");
// ...
Scheduler::sched();
// ...
println!("Print, but possibly crash");
@redbaron

This comment has been minimized.

Copy link
Contributor

redbaron commented Jul 20, 2016

seems there is no short-term workaround either? I think it should be mentioned in README in both coio-rs and mioco.

@zonyitoo

This comment has been minimized.

Copy link
Owner Author

zonyitoo commented Jul 24, 2016

It seems that someone is going to add coroutine support directly to LLVM, which means that it is possible to tell LLVM not to inline TLS calls between context swaps.

https://internals.rust-lang.org/t/llvm-coroutines-to-bring-awarness

@ihrwein

This comment has been minimized.

Copy link

ihrwein commented Jun 16, 2017

First of all, thanks for the great work what you put into this library. I played with setjmp/longjmp to have simple generators in Rust, then found ucontext and ultimately this crate. My intention was to have Go like coroutines in Rust.

I tried to compile coio but one if its dependency became outdated and today's Rust compiler won't accept it (owning_ref, maybe).

I read the long issue in the std library that TLS should be eliminated from std to support coroutines. I don't have the knowledge to add any value to that conversation, but FYI LLVM 4.0 has experimental coroutine support, which may help.

Somebody wrote that TLS is just a hidden extra parameter to functions. According to this link, an extra .tdata segment is created in the executable and each thread will get a different copy from this.

I may have another idea: if a TLS is accessed from a coroutine, then the TLS is not placed into .tdata but into the coroutine's captured environment. What do you think about this approach? Is it feasible?

@redbaron

This comment has been minimized.

Copy link
Contributor

redbaron commented Jun 16, 2017

If Goroutines were in Rust it would be a pure awesomeness

@jedisct1

This comment has been minimized.

Copy link
Contributor

jedisct1 commented Jun 16, 2017

futures-await goes in this direction.

@jedisct1

This comment has been minimized.

Copy link
Contributor

jedisct1 commented Jun 16, 2017

@zonyitoo

This comment has been minimized.

Copy link
Owner Author

zonyitoo commented Jun 17, 2017

@ihrwein Thanks for your suggestion. What you are proposing is "coroutine local variable", which stores data with the Coroutine. But as you can see, it requires Rust's official teams' support, and they don't want to provide any.

@lhecker

This comment has been minimized.

Copy link
Collaborator

lhecker commented Jun 17, 2017

I wrote down my thoughts about this here: rust-lang/rfcs#2033 (comment)
tl;dr: I see the future of coio etc. a bit pessimistic and fully expect a mediocre solution consisting out of async/await and tokio to be the official standard in the future.

@aep

This comment has been minimized.

Copy link

aep commented Jul 6, 2018

Isn't this a matter of just not inlining TLS access?

Might be a little too late here since rust is headed to standardizing futures, but I wrote an entire linker https://github.com/aep/elfkit which can easily be adapted to support a different TLS strategy.

@zonyitoo

This comment has been minimized.

Copy link
Owner Author

zonyitoo commented Jul 6, 2018

Yup. I have already figure out. This is nothing to do with the TLS inlining.
But I don't have time to maintain this project anymore, so this issue keep hanging.

@zonyitoo

This comment has been minimized.

Copy link
Owner Author

zonyitoo commented Aug 27, 2018

PANIC_COUNT issue in #70

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.