coio-rs icon indicating copy to clipboard operation
coio-rs copied to clipboard

Bug: Incompatibility of Rust's stdlib

Open zonyitoo opened this issue 8 years ago • 13 comments

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 https://github.com/rust-lang/rust/pull/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.

zonyitoo avatar May 15 '16 07:05 zonyitoo

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

redbaron avatar Jul 19 '16 21:07 redbaron

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");

zonyitoo avatar Jul 19 '16 23:07 zonyitoo

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

redbaron avatar Jul 20 '16 08:07 redbaron

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

zonyitoo avatar Jul 24 '16 00:07 zonyitoo

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?

ihrwein avatar Jun 16 '17 10:06 ihrwein

If Goroutines were in Rust it would be a pure awesomeness

redbaron avatar Jun 16 '17 13:06 redbaron

futures-await goes in this direction.

jedisct1 avatar Jun 16 '17 13:06 jedisct1

Also: https://github.com/rust-lang/rfcs/pull/2033

jedisct1 avatar Jun 16 '17 13:06 jedisct1

@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.

zonyitoo avatar Jun 17 '17 10:06 zonyitoo

I wrote down my thoughts about this here: https://github.com/rust-lang/rfcs/pull/2033#issuecomment-309212006 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.

lhecker avatar Jun 17 '17 12:06 lhecker

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.

aep avatar Jul 06 '18 07:07 aep

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 avatar Jul 06 '18 10:07 zonyitoo

PANIC_COUNT issue in #70

zonyitoo avatar Aug 27 '18 05:08 zonyitoo