Skip to content

Commit ce31703

Browse files
committed
fix: improve motivation and explanation for the change of channels
1 parent 82130e0 commit ce31703

File tree

1 file changed

+61
-9
lines changed

1 file changed

+61
-9
lines changed

text/0000-forget-marker-trait.md

Lines changed: 61 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -321,10 +321,14 @@ fn weakener<T>(foo: T) -> i32 {
321321
}
322322
```
323323

324-
## Not only forgets
324+
## How API of channels needs to change in order to work with `!Forget` types.
325325
[channels-unsoundness]: #channels-unsoundness
326326

327-
There exists a way to exploit the old `thread::scoped` API without any memory leaks! We can move `JoinHandle` inside the thread it is meant to protect, creating a kind of cycle:
327+
Currently, channels are created via `let (tx, rx) = channel()`. This is not compatible with `!Forget` types. This section explains how developers should use them instead and why.
328+
329+
There exists a way to exploit the old `thread::scoped` API without any memory leaks[^no_leaks_sidenode]. We can move `JoinHandle` inside the thread it is meant to protect, thereby creating a cyclic relationship:
330+
331+
[^no_leaks_sidenode]: This would become a memory leak if instead of spawning the thread will use another way to create the cycle. See later.
328332

329333
```rust
330334
use std::{
@@ -344,7 +348,7 @@ where
344348
F: FnOnce() -> (),
345349
F: Send + 'a,
346350
{
347-
todo!()
351+
todo!("schedule `F` on the actual thread")
348352
}
349353

350354
fn main() {
@@ -370,7 +374,7 @@ fn main() {
370374
}
371375
```
372376

373-
In this code, no memory is leaked, and `JoinHandle`'s destructor is not skipped. However, many types of channels - including rendezvous channelscan also be vulnerable to this issue if their signatures allow an equivalent implementation using reference counting.
377+
This code is clearly unsound because we are aliasing a mutable reference, which permits potential data races and use-after-free issues. Furthermore, many types of channels - including rendezvous channelscan be vulnerable to this issue if their signatures allow an equivalent implementation using reference counting.
374378

375379
```rust
376380
fn main() {
@@ -394,10 +398,58 @@ fn main() {
394398
}
395399
```
396400

401+
What is the actual problem?
402+
403+
### The reason of the channels unsoundness
404+
[channels-unsoundness-reason]: #channels-unsoundness-reason
405+
406+
The core issue is not inherent to `scoped` or `JoinHandle` per se - it lies in the API design and its interaction with `!Forget` types. From the type system's perspective, `scoped` is consuming `F` and returning another type with some lifetime. This ereasure plays a critical role to avoid a cyclic type that will not compile. It creates a pathway for unsoundness when combined with signatures resembling reference-counted types like `Arc`.
407+
408+
```rust
409+
trait Erase { }
410+
impl<T> Erase for T {}
411+
412+
struct JoinHandle<'a>(Box<dyn Erase + Send + 'a>);
413+
414+
impl Drop for JoinHandle<'_> {
415+
fn drop(&mut self) {}
416+
}
417+
418+
fn scoped<'a, F>(f: F) -> JoinHandle<'a>
419+
where
420+
F: FnOnce() -> (),
421+
F: Send + 'a,
422+
{
423+
JoinHandle(Box::new(f))
424+
}
425+
426+
fn main() {
427+
let (tx, rx) = std::sync::mpsc::channel();
428+
429+
let mut buf = [0; 1024];
430+
let buf_ref = &mut buf;
431+
432+
let handle = scoped(move || {
433+
buf_ref[0] = 1;
434+
drop(rx);
435+
});
436+
437+
_ = tx.send(handle);
438+
drop(tx);
439+
440+
// handle no longer guards `buf`.
441+
442+
// "aliased" `&mut`, but it is not UB in that case, because `f` in not running.
443+
buf[0] = 1;
444+
}
445+
```
446+
397447
### Solution for message passing of `!Forget` types.
398448
[solution-to-self-referential-problem]: #solution-to-self-referential-problem
399449

400-
One might speculate and try to fix some holes, for example by making `JoinHandle: !Send`, but this can only count as a workaround. If we look at the problem in depth, we can see that `Forget` is generally incompatible with `Rc`, as well as other APIs that can be expressed with its signature, because it creates a hidden self-reference. In the example earlier, the borrow checker cannot see a connection between `rx` and `tx` - when `tx` is dropped, `buf` is no longer borrowed. What if retained such a connection?
450+
Thus, there is no point in blaming `scoped`; its API can already be replicated safely in current code, and other `Arc`-like APIs can also cause leaks. This demonstrates that approaches such as making `JoinHandle: !Send` are not feasible. How can we fix it?
451+
452+
Looking at our first example with `Arc`, let's replace our `Arc` with a reference:
401453

402454
```rust
403455
fn main() {
@@ -421,7 +473,7 @@ fn main() {
421473
}
422474
```
423475

424-
And we got a compiler error preventing the unsoundness:
476+
This change results in compiler errors that prevent the unsound behavior:
425477

426478
```rust
427479
error[E0597]: `mutex` does not live long enough
@@ -453,7 +505,7 @@ error[E0505]: cannot move out of `mutex` because it is borrowed
453505
| borrow later used here
454506
```
455507

456-
This example is exactly like the first one with `Arc`, but uses references instead - we are allowed to pass them with `JoinHandle: !Forget`. But what with channels? There are not so many examples in the ecosystem that follow this approach in the signature, as it is not `'static`, but there are some:
508+
Here, a single action to replace `Arc<T>` with `&'a T` allowed code to become sound - `JoinHandle: !Forget` allowes to pass references into tasks and removes the need for reference counting in that case. The same approach applies to channels. This approach is not compatible with `JoinHandle: Forget` and cannot be used today with functions like `tokio::spawn` due to `'static` bound, but ecosystem has some examples:
457509

458510
```rust
459511
fn main() {
@@ -478,9 +530,9 @@ fn main() {
478530
}
479531
```
480532

481-
This code fails to compile too. Why? Because borrow checker detects a self-reference! `handle` borrows `queue`, but we are moving `handle` into `queue`, thus `queue` borrows `queue`. This means we cannot call `drop` on queue or take a reference to it, but since `drop` is inserted by the compiler, we have an error. If there was a `loop {}` and borrow cheker considered diverging during analysis, it would compile and would be sound.
533+
This code fails to compile, which prevents the unsound behavior. The failure occurs because the borrow checker detects a self-reference: `handle` borrows `queue`, but moving `handle` into `queue` causes `queue` to indirectly borrow itself. Since the compiler inserts a call to `drop` on `queue`, this self-referential borrow is caught at compile time.
482534

483-
This means that to use message-passing with `!Forget` types, API authors must rely on lifetimes more - because `Forget` types fundamentally involve lifetime management. Looking at the example above, `rx` cannot be passed to the traditional `spawn`, because of the `F: 'static` requirement. But `thread::scope` allows it - as well as async `scope` does, with the future itself being `!Forget`. Note that rendezvous channels can be soundly expressed using that API and `PhantomData`.
535+
Thus, to support message passing with `!Forget` types, API authors must rely more heavily on lifetimes. Since `Forget` types inherently involve lifetime management, using explicit lifetimes (for example, by replacing `Arc<T>` with `&'a T`, having `PhantomData` together with a pointer etc) prevents the formation of cycles that can lead to unsoundness. While this approach is not compatible with APIs that require a `'static` bound (such as `tokio::spawn`), it works in environments like `thread::scope` or async scopes, where the future itself can be `!Forget`. Notably, rendezvous channels can be soundly expressed using this API alongside `PhantomData`.
484536

485537
## Traditional combinators and patterns
486538
[traditional-workflows]: #traditional-workflows

0 commit comments

Comments
 (0)