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

Unsoundness mapping/downgrading in RwLock #3344

Closed
Kestrer opened this issue Dec 25, 2020 · 2 comments · Fixed by #3345
Closed

Unsoundness mapping/downgrading in RwLock #3344

Kestrer opened this issue Dec 25, 2020 · 2 comments · Fixed by #3345
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug. I-unsound 💥 A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness M-sync Module: tokio/sync

Comments

@Kestrer
Copy link
Contributor

Kestrer commented Dec 25, 2020

Version: Tokio 1.0.0

In the code:

use std::cell::Cell;
use tokio::sync::{RwLock, RwLockWriteGuard};

#[tokio::main]
async fn main() {
    let lock = RwLock::new(Cell::new(10_u32));
    let guard = lock.write().await;
    let guard = RwLockWriteGuard::map(guard, Cell::get_mut);
    let guard = RwLockWriteGuard::downgrade(guard);
    let value_ref: &u32 = &*guard;
    lock.read().await.set(11);
        
    drop(value_ref);
}

We obtain a reference to the Cell as well as a reference to the inner value - this can be seen in the error running Miri on the playground.

This same unsoundness occurred in parking_lot, and they had to remove the downgrade API (on mapped guards) as a result. However, this isn't really an option for Tokio now that it's stable.

(sorry for the edits, I accidentally posted this before I'd finished writing it)

@Kestrer Kestrer added A-tokio Area: The main tokio crate C-bug Category: This is a bug. labels Dec 25, 2020
@Kestrer Kestrer changed the title Unsoundness mappipngin RwLock Unsoundness mapping/downgrading in RwLock Dec 25, 2020
@Darksonn Darksonn added the M-sync Module: tokio/sync label Dec 25, 2020
@Darksonn
Copy link
Contributor

Well that's problematic. It was added in #2733.

@MikailBag
Copy link
Contributor

AFAIK fixing soundness bug is valid reason to break backwards compatibility.
For example Rust RFC 1122 allows it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug. I-unsound 💥 A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness M-sync Module: tokio/sync
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants