tokio-rs / async-stream

Asynchronous streams for Rust using async & await notation
Other
636 stars 33 forks source link

Fix unsoundness issues #106 and #107 #109

Closed aumetra closed 1 month ago

aumetra commented 4 months ago

This PR fixes the unsoundness reported in #106 and #107

cc @steffahn (for checking the fixes, whether they are actually correct)

Closes #106 Closes #107

aumetra commented 4 months ago

The CI failures seem unrelated. Probably UI updates in the Rust compiler. Should I fix them in this PR or separately?

taiki-e commented 1 month ago

Thanks for the PR!

aumetra commented 1 month ago

Of course, will do

taiki-e commented 1 month ago

It seems that the MSRV bump triggered a new clippy warning. Could you address that as well?

error: initializer for `thread_local` value can be made `const`
  --> async-stream/src/yielder.rs:37:45
   |
37 | thread_local!(static STORE: Cell<*mut ()> = Cell::new(ptr::null_mut()));
   |                                             ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `const { Cell::new(ptr::null_mut()) }`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#missing_const_for_thread_local
   = note: `-D clippy::missing-const-for-thread-local` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(clippy::missing_const_for_thread_local)]`
aumetra commented 1 month ago

I mean, I just allowed the clippy lint for now since the MSRV definitely doesn't have support for inline const

taiki-e commented 1 month ago

inline const in thread_local is special and is available since Rust 1.59: https://github.com/rust-lang/rust/pull/91355

aumetra commented 1 month ago

Ah, interesting. Didn't know. Fixing it.

taiki-e commented 1 month ago

Published in 0.3.6.