Prevent edge case race condition, hopefully

coolreader18 · coolreader18 · commit af7ec416f90b · 2021-11-15T16:08:47.000-06:00
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/common/Cargo.toml b/common/Cargo.toml
@@ -20,7 +20,7 @@ once_cell = "1.4.1"
 siphasher = "0.3"
 rand = "0.8"
 volatile = "0.3"
-radium = "0.6"
+radium = "0.7"
 libc = "0.2.101"
 ascii = "1.0"
 unic-ucd-category = "0.9"
diff --git a/common/src/refcount.rs b/common/src/refcount.rs
@@ -38,6 +38,14 @@ impl RefCount {
         }
     }
 
+    /// Returns true if successful
+    #[inline]
+    pub fn safe_inc(&self) -> bool {
+        self.strong
+            .fetch_update(AcqRel, Acquire, |prev| (prev != 0).then(|| prev + 1))
+            .is_ok()
+    }
+
     /// Decrement the reference count. Returns true when the refcount drops to 0.
     #[inline]
     pub fn dec(&self) -> bool {
diff --git a/vm/src/pyobjectrc.rs b/vm/src/pyobjectrc.rs
@@ -314,11 +314,12 @@ cfg_if::cfg_if! {
 
 impl PyWeak {
     pub(crate) fn upgrade(&self) -> Option<PyObjectRef> {
-        // TODO: figure out orderings for here, in drop, and the store in WeakRefList::clear
         let guard = unsafe { self.parent.as_ref().lock() };
         let obj_ptr = guard.obj?;
         unsafe {
-            obj_ptr.as_ref().0.refcount.inc();
+            if !obj_ptr.as_ref().0.refcount.safe_inc() {
+                return None;
+            }
             Some(PyObjectRef::from_raw(obj_ptr.as_ptr()))
         }
     }

Original file line number	Diff line number	Diff line change
`@@ -38,6 +38,14 @@ impl RefCount {`
`38`	`38`	`}`
`39`	`39`	`}`
`40`	`40`
	`41`	`+ /// Returns true if successful`
	`42`	`+ #[inline]`
	`43`	`+ pub fn safe_inc(&self) -> bool {`
	`44`	`+ self.strong`
	`45`	`+ .fetch_update(AcqRel, Acquire, \|prev\| (prev != 0).then(\|\| prev + 1))`
	`46`	`+ .is_ok()`
	`47`	`+ }`
	`48`	`+`
`41`	`49`	`/// Decrement the reference count. Returns true when the refcount drops to 0.`
`42`	`50`	`#[inline]`
`43`	`51`	`pub fn dec(&self) -> bool {`
Original file line number	Diff line number	Diff line change
`@@ -314,11 +314,12 @@ cfg_if::cfg_if! {`
`314`	`314`
`315`	`315`	`impl PyWeak {`
`316`	`316`	`pub(crate) fn upgrade(&self) -> Option<PyObjectRef> {`
`317`		`- // TODO: figure out orderings for here, in drop, and the store in WeakRefList::clear`
`318`	`317`	`let guard = unsafe { self.parent.as_ref().lock() };`
`319`	`318`	`let obj_ptr = guard.obj?;`
`320`	`319`	`unsafe {`
`321`		`- obj_ptr.as_ref().0.refcount.inc();`
	`320`	`+ if !obj_ptr.as_ref().0.refcount.safe_inc() {`
	`321`	`+ return None;`
	`322`	`+ }`
`322`	`323`	`Some(PyObjectRef::from_raw(obj_ptr.as_ptr()))`
`323`	`324`	`}`
`324`	`325`	`}`