Skip to content

Commit 66c8984

Browse files
committed
std: Fix Rc with Gc inside of it
The code in Rc assumes that the inner box is allocated on the global exchange heap, which is not always true. If T has managed pointers inside of it, it's allocate on the local heap instead. This commit reconciles these two modes of T by carefully interacting with the inner box (with special treatment of its deallocation). Closes rust-lang#11532
1 parent 480b0f4 commit 66c8984

File tree

1 file changed

+65
-37
lines changed

1 file changed

+65
-37
lines changed

src/libstd/rc.rs

Lines changed: 65 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ use cast::transmute;
2727
use ops::Drop;
2828
use cmp::{Eq, Ord};
2929
use clone::{Clone, DeepClone};
30-
use rt::global_heap::exchange_free;
3130
use ptr::read_ptr;
3231
use option::{Option, Some, None};
3332

@@ -57,30 +56,38 @@ impl<T> Rc<T> {
5756
/// Borrow the value contained in the reference-counted box
5857
#[inline(always)]
5958
pub fn borrow<'a>(&'a self) -> &'a T {
60-
unsafe { &(*self.ptr).value }
59+
&self.inner().value
6160
}
6261

6362
/// Downgrade the reference-counted pointer to a weak reference
6463
pub fn downgrade(&self) -> Weak<T> {
65-
unsafe {
66-
(*self.ptr).weak += 1;
67-
Weak { ptr: self.ptr }
68-
}
64+
self.inner().weak += 1;
65+
Weak { ptr: self.ptr }
66+
}
67+
68+
fn inner<'a>(&'a self) -> &'a mut RcBox<T> {
69+
// Note that we need the indrection of &~RcBox<T> because we can't
70+
// transmute *RcBox to &RcBox (the actual pointer layout is different if
71+
// T contains managed pointers at this time)
72+
let ptr: &mut ~RcBox<T> = unsafe { transmute(&self.ptr) };
73+
&mut **ptr
6974
}
7075
}
7176

7277
#[unsafe_destructor]
7378
impl<T> Drop for Rc<T> {
7479
fn drop(&mut self) {
75-
unsafe {
76-
if self.ptr != 0 as *mut RcBox<T> {
77-
(*self.ptr).strong -= 1;
78-
if (*self.ptr).strong == 0 {
79-
read_ptr(self.borrow()); // destroy the contained object
80-
if (*self.ptr).weak == 0 {
81-
exchange_free(self.ptr as *mut u8 as *i8)
82-
}
83-
}
80+
if self.ptr == 0 as *mut RcBox<T> { return }
81+
82+
let inner = self.inner();
83+
inner.strong -= 1;
84+
if inner.strong == 0 {
85+
// If we've run out of strong pointers, we need to be sure to run
86+
// the destructor *now*, but we can't free the value just yet (weak
87+
// pointers may still be active).
88+
unsafe { read_ptr(&inner.value); } // destroy the contained object
89+
if inner.weak == 0 {
90+
free(self.ptr);
8491
}
8592
}
8693
}
@@ -89,10 +96,8 @@ impl<T> Drop for Rc<T> {
8996
impl<T> Clone for Rc<T> {
9097
#[inline]
9198
fn clone(&self) -> Rc<T> {
92-
unsafe {
93-
(*self.ptr).strong += 1;
94-
Rc { ptr: self.ptr }
95-
}
99+
self.inner().strong += 1;
100+
Rc { ptr: self.ptr }
96101
}
97102
}
98103

@@ -135,41 +140,56 @@ pub struct Weak<T> {
135140
impl<T> Weak<T> {
136141
/// Upgrade a weak reference to a strong reference
137142
pub fn upgrade(&self) -> Option<Rc<T>> {
138-
unsafe {
139-
if (*self.ptr).strong == 0 {
140-
None
141-
} else {
142-
(*self.ptr).strong += 1;
143-
Some(Rc { ptr: self.ptr })
144-
}
143+
if self.inner().strong == 0 {
144+
None
145+
} else {
146+
self.inner().strong += 1;
147+
Some(Rc { ptr: self.ptr })
145148
}
146149
}
150+
151+
fn inner<'a>(&'a self) -> &'a mut RcBox<T> {
152+
// see above version
153+
let ptr: &mut ~RcBox<T> = unsafe { transmute(&self.ptr) };
154+
&mut **ptr
155+
}
147156
}
148157

149158
#[unsafe_destructor]
150159
impl<T> Drop for Weak<T> {
151160
fn drop(&mut self) {
152-
unsafe {
153-
if self.ptr != 0 as *mut RcBox<T> {
154-
(*self.ptr).weak -= 1;
155-
if (*self.ptr).weak == 0 && (*self.ptr).strong == 0 {
156-
exchange_free(self.ptr as *mut u8 as *i8)
157-
}
158-
}
161+
if self.ptr as uint == 0 { return }
162+
163+
let inner = self.inner();
164+
inner.weak -= 1;
165+
if inner.weak == 0 && inner.strong == 0 {
166+
free(self.ptr);
159167
}
160168
}
161169
}
162170

163171
impl<T> Clone for Weak<T> {
164172
#[inline]
165173
fn clone(&self) -> Weak<T> {
166-
unsafe {
167-
(*self.ptr).weak += 1;
168-
Weak { ptr: self.ptr }
169-
}
174+
self.inner().weak += 1;
175+
Weak { ptr: self.ptr }
170176
}
171177
}
172178

179+
// We need to be very careful when dropping this inner box pointer. We don't
180+
// necessarily know the right free function to call because it's different
181+
// depending on whether T contains managed pointers or not. The other sticky
182+
// part is that we can't run the destructor for T because it's already been run.
183+
//
184+
// To get around this, we transmute the pointer to an owned pointer with a type
185+
// that has size 0. This preserves the managed-ness of the type along with
186+
// preventing any destructors from being run. Note that this assumes that the GC
187+
// doesn't need to know the real size of the pointer (it's just malloc right
188+
// now), so this works for now but may need to change in the future.
189+
fn free<T>(ptr: *mut RcBox<T>) {
190+
let _: ~RcBox<[T, ..0]> = unsafe { transmute(ptr) };
191+
}
192+
173193
#[cfg(test)]
174194
mod tests {
175195
use prelude::*;
@@ -230,4 +250,12 @@ mod tests {
230250
drop(x);
231251
assert!(y.upgrade().is_none());
232252
}
253+
254+
#[test]
255+
fn gc_inside() {
256+
// see issue #11532
257+
use {cell, gc};
258+
let a = Rc::new(cell::RefCell::new(gc::Gc::new(1)));
259+
assert!(a.borrow().try_borrow_mut().is_some());
260+
}
233261
}

0 commit comments

Comments
 (0)