Bug 1794645 - Use usize for refcounting non-atomic Rust XPCOM objects. r=nika

This matches the implementation of refcounting in C++ XPCOM objects.
We'll end up crashing if the refcount being returned is too large.

Change inc() and dec() to use try_into instead of as. This will change
the behavior because we'll crash if the value is too large instead of
silently truncating the return value, which might be worse because few
callers actually use the return value, but is unlikely to matter.

Also, use a more precise return type for the get() methods. It is only
used by Rust code, so there's no need to be compatible with
nsISupports. I made that change here to avoid needing to add a coercion
from usize to nsrefcnt.

Part of my goal with this patch is to eliminate an unnecessary use of
nsrefcnt from Rust.

Differential Revision: https://phabricator.services.mozilla.com/D159124
This commit is contained in:
Andrew McCreight 2022-10-13 14:53:08 +00:00
Родитель 24639267ae
Коммит cdd79677c6
1 изменённых файлов: 11 добавлений и 9 удалений

Просмотреть файл

@ -6,6 +6,7 @@ use crate::interfaces::{nsISupports, nsrefcnt};
use libc;
use nserror::{nsresult, NS_OK};
use std::cell::Cell;
use std::convert::TryInto;
use std::fmt;
use std::marker::PhantomData;
use std::mem;
@ -251,7 +252,7 @@ where
///
/// `#[xpcom(nonatomic)]` will use this type for the `__refcnt` field.
#[derive(Debug)]
pub struct Refcnt(Cell<nsrefcnt>);
pub struct Refcnt(Cell<usize>);
impl Refcnt {
/// Create a new reference count value. This is unsafe as manipulating
/// Refcnt values is an easy footgun.
@ -265,7 +266,7 @@ impl Refcnt {
// XXX: Checked add?
let new = self.0.get() + 1;
self.0.set(new);
new
new.try_into().unwrap()
}
/// Decrement the reference count. Returns the new reference count. This is
@ -274,11 +275,11 @@ impl Refcnt {
// XXX: Checked sub?
let new = self.0.get() - 1;
self.0.set(new);
new
new.try_into().unwrap()
}
/// Get the current value of the reference count.
pub fn get(&self) -> nsrefcnt {
pub fn get(&self) -> usize {
self.0.get()
}
}
@ -301,13 +302,14 @@ impl AtomicRefcnt {
/// Increment the reference count. Returns the new reference count. This is
/// unsafe as modifying this value can cause a use-after-free.
pub unsafe fn inc(&self) -> nsrefcnt {
self.0.fetch_add(1, Ordering::Relaxed) as nsrefcnt + 1
let result = self.0.fetch_add(1, Ordering::Relaxed) + 1;
result.try_into().unwrap()
}
/// Decrement the reference count. Returns the new reference count. This is
/// unsafe as modifying this value can cause a use-after-free.
pub unsafe fn dec(&self) -> nsrefcnt {
let result = self.0.fetch_sub(1, Ordering::Release) as nsrefcnt - 1;
let result = self.0.fetch_sub(1, Ordering::Release) - 1;
if result == 0 {
// We're going to destroy the object on this thread, so we need
// acquire semantics to synchronize with the memory released by
@ -322,12 +324,12 @@ impl AtomicRefcnt {
atomic::fence(Ordering::Acquire);
}
}
result
result.try_into().unwrap()
}
/// Get the current value of the reference count.
pub fn get(&self) -> nsrefcnt {
self.0.load(Ordering::Acquire) as nsrefcnt
pub fn get(&self) -> usize {
self.0.load(Ordering::Acquire)
}
}