rv6 icon indicating copy to clipboard operation
rv6 copied to clipboard

`RcCell` panics on drop?

Open Medowhill opened this issue 3 years ago • 2 comments

현재 RcCell은 rc가 0보다 큰 상황에서 드롭되면 패닉을 발생시킵니다. rv6에서는 RcCell을 스택에 배치하는 경우가 없고 한 스레드라도 패닉하면 결국 커널 전체가 패닉하므로 이 설계가 큰 문제가 아닐 수 있지만, 일반적인 경우를 생각해 보면 현재의 RcCell 설계는 메모리 안전성을 깰 수 있어 보입니다.

아래 코드에서 RcCell을 소유한 스레드가 종료되면서 RcCell이 드롭되면 rc가 0이 아니기 때문에 패닉이 발생합니다. 하지만 메인 스레드의 실행에는 영향을 주지 않기 때문에, 이미 드롭된 RcCell을 가리키는 Ref를 dereference할 수 있으며 이는 UB입니다(실제 실행해 보면 segmentaion fault가 발생하며 Miri로도 UB가 탐지됨).

https://doc.rust-lang.org/std/ops/trait.Drop.html#panics에 따르면 drop안에서 패닉이 발생하더라도 그 값은 드롭된 것으로 간주되는 것 같습니다.

Note that even if this panics, the value is considered to be dropped;

use std::{ops::Deref, pin::Pin, sync::{atomic::{AtomicUsize, Ordering}, mpsc::channel}, thread};

fn main() {
    let (tx, rx) = channel();

    let handler = thread::spawn(move || {
        let rc = Rc::new(0u32);
        let rc = Pin::new(&rc);
        let r = rc.borrow();
        tx.send(r).unwrap();
    });

    let _ = handler.join();
    let r = rx.recv().unwrap();
    let _: &u32 = &r;
}

struct Rc<T> {
    data: T,
    rc: AtomicUsize,
}

impl<T> Rc<T> {
    const fn new(data: T) -> Self {
        Self {
            data,
            rc: AtomicUsize::new(0),
        }
    }

    fn borrow(self: Pin<&Self>) -> Ref<T> {
        self.rc.fetch_add(1, Ordering::SeqCst);
        Ref(self.get_ref())
    }
}

impl<T> Drop for Rc<T> {
    fn drop(&mut self) {
        if self.rc.load(Ordering::SeqCst) > 0 {
            panic!();
        }
    }
}

struct Ref<T>(*const Rc<T>);

unsafe impl<T: Send> Send for Ref<T> {}
unsafe impl<T: Send> Sync for Ref<T> {}

impl<T> Deref for Ref<T> {
    type Target = T;

    fn deref(&self) -> &Self::Target {
        unsafe { &(*self.0).data }
    }
}

impl<T> Drop for Ref<T> {
    fn drop(&mut self) {
        unsafe { &(*self.0).rc }.fetch_sub(1, Ordering::SeqCst);
    }
}

Medowhill avatar May 28 '21 04:05 Medowhill

Alternative design 1:

impl<T> Drop for Rc<T> {
    fn drop(&mut self) {
        if self.rc.load(Ordering::SeqCst) > 0 {
            loop {}
        }
    }
}

Alternative design 2:

impl<T> Drop for Rc<T> {
    fn drop(&mut self) {
        while self.rc.load(Ordering::SeqCst) > 0 {}
    }
}

Medowhill avatar Jun 03 '21 06:06 Medowhill

지금 논문에 쓴 ArcCellStaticArc 디자인이 좀 다른 것 같습니다. 일반적으로 우리가 디자인한 모든 것에 대해 논문에 쓴대로 kernel code도 수정해야할 것 같습니다.

jeehoonkang avatar Aug 23 '21 14:08 jeehoonkang