Auto merge of #26792 - servo:static-mut, r=nox

Replace `static mut` with `const`, `static`+`AtomicPtr`, or `static`+`UnsafeCell`

Fixes https://github.com/servo/servo/issues/26550
This commit is contained in:
bors-servo 2020-06-05 09:56:41 -04:00 committed by GitHub
commit 00b57b4fd9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 93 additions and 77 deletions

View file

@ -23,12 +23,19 @@ use unwind_sys::{
#[link(name = "lzma")] #[link(name = "lzma")]
extern "C" {} extern "C" {}
static mut SHARED_STATE: SharedState = SharedState { struct UncheckedSyncUnsafeCell<T>(std::cell::UnsafeCell<T>);
msg2: None,
msg3: None, /// Safety: dereferencing the pointer from `UnsafeCell::get` must involve external synchronization
msg4: None, unsafe impl<T> Sync for UncheckedSyncUnsafeCell<T> {}
context: AtomicPtr::new(ptr::null_mut()),
}; static SHARED_STATE: UncheckedSyncUnsafeCell<SharedState> =
UncheckedSyncUnsafeCell(std::cell::UnsafeCell::new(SharedState {
msg2: None,
msg3: None,
msg4: None,
}));
static CONTEXT: AtomicPtr<libc::ucontext_t> = AtomicPtr::new(ptr::null_mut());
type MonitoredThreadId = libc::pid_t; type MonitoredThreadId = libc::pid_t;
@ -37,25 +44,30 @@ struct SharedState {
msg2: Option<PosixSemaphore>, msg2: Option<PosixSemaphore>,
msg3: Option<PosixSemaphore>, msg3: Option<PosixSemaphore>,
msg4: Option<PosixSemaphore>, msg4: Option<PosixSemaphore>,
context: AtomicPtr<libc::ucontext_t>,
} }
fn clear_shared_state() { fn clear_shared_state() {
// Safety: this is only called from the sampling thread (theres only one)
// Sampled threads only access SHARED_STATE in their signal handler.
// This signal and the semaphores in SHARED_STATE provide the necessary synchronization.
unsafe { unsafe {
SHARED_STATE.msg2 = None; let shared_state = &mut *SHARED_STATE.0.get();
SHARED_STATE.msg3 = None; shared_state.msg2 = None;
SHARED_STATE.msg4 = None; shared_state.msg3 = None;
SHARED_STATE.context = AtomicPtr::new(ptr::null_mut()); shared_state.msg4 = None;
} }
CONTEXT.store(ptr::null_mut(), Ordering::SeqCst);
} }
fn reset_shared_state() { fn reset_shared_state() {
// Safety: same as clear_shared_state
unsafe { unsafe {
SHARED_STATE.msg2 = Some(PosixSemaphore::new(0).expect("valid semaphore")); let shared_state = &mut *SHARED_STATE.0.get();
SHARED_STATE.msg3 = Some(PosixSemaphore::new(0).expect("valid semaphore")); shared_state.msg2 = Some(PosixSemaphore::new(0).expect("valid semaphore"));
SHARED_STATE.msg4 = Some(PosixSemaphore::new(0).expect("valid semaphore")); shared_state.msg3 = Some(PosixSemaphore::new(0).expect("valid semaphore"));
SHARED_STATE.context = AtomicPtr::new(ptr::null_mut()); shared_state.msg4 = Some(PosixSemaphore::new(0).expect("valid semaphore"));
} }
CONTEXT.store(ptr::null_mut(), Ordering::SeqCst);
} }
struct PosixSemaphore { struct PosixSemaphore {
@ -194,16 +206,18 @@ impl Sampler for LinuxSampler {
// signal the thread, wait for it to tell us state was copied. // signal the thread, wait for it to tell us state was copied.
send_sigprof(self.thread_id); send_sigprof(self.thread_id);
unsafe {
SHARED_STATE
.msg2
.as_ref()
.unwrap()
.wait_through_intr()
.expect("msg2 failed");
}
let context = unsafe { SHARED_STATE.context.load(Ordering::SeqCst) }; // Safety: non-exclusive reference only
// since sampled threads are accessing this concurrently
let shared_state = unsafe { &*SHARED_STATE.0.get() };
shared_state
.msg2
.as_ref()
.unwrap()
.wait_through_intr()
.expect("msg2 failed");
let context = CONTEXT.load(Ordering::SeqCst);
let mut cursor = mem::MaybeUninit::uninit(); let mut cursor = mem::MaybeUninit::uninit();
let ret = unsafe { unw_init_local(cursor.as_mut_ptr(), context) }; let ret = unsafe { unw_init_local(cursor.as_mut_ptr(), context) };
let result = if ret == UNW_ESUCCESS { let result = if ret == UNW_ESUCCESS {
@ -231,24 +245,23 @@ impl Sampler for LinuxSampler {
}; };
// signal the thread to continue. // signal the thread to continue.
unsafe { shared_state
SHARED_STATE .msg3
.msg3 .as_ref()
.as_ref() .unwrap()
.unwrap() .post()
.post() .expect("msg3 failed");
.expect("msg3 failed");
}
// wait for thread to continue. // wait for thread to continue.
unsafe { shared_state
SHARED_STATE .msg4
.msg4 .as_ref()
.as_ref() .unwrap()
.unwrap() .wait_through_intr()
.wait_through_intr() .expect("msg4 failed");
.expect("msg4 failed");
} // No-op, but marks the end of the shared borrow
drop(shared_state);
clear_shared_state(); clear_shared_state();
@ -271,26 +284,27 @@ extern "C" fn sigprof_handler(
ctx: *mut libc::c_void, ctx: *mut libc::c_void,
) { ) {
assert_eq!(sig, libc::SIGPROF); assert_eq!(sig, libc::SIGPROF);
unsafe { // copy the context.
// copy the context. CONTEXT.store(ctx as *mut libc::ucontext_t, Ordering::SeqCst);
SHARED_STATE
.context
.store(ctx as *mut libc::ucontext_t, Ordering::SeqCst);
// Tell the sampler we copied the context.
SHARED_STATE.msg2.as_ref().unwrap().post().expect("posted");
// Wait for sampling to finish. // Safety: non-exclusive reference only
SHARED_STATE // since the sampling thread is accessing this concurrently
.msg3 let shared_state = unsafe { &*SHARED_STATE.0.get() };
.as_ref()
.unwrap()
.wait_through_intr()
.expect("msg3 wait succeeded");
// OK we are done! // Tell the sampler we copied the context.
SHARED_STATE.msg4.as_ref().unwrap().post().expect("posted"); shared_state.msg2.as_ref().unwrap().post().expect("posted");
// DO NOT TOUCH shared state here onwards.
} // Wait for sampling to finish.
shared_state
.msg3
.as_ref()
.unwrap()
.wait_through_intr()
.expect("msg3 wait succeeded");
// OK we are done!
shared_state.msg4.as_ref().unwrap().post().expect("posted");
// DO NOT TOUCH shared state here onwards.
} }
fn send_sigprof(to: libc::pid_t) { fn send_sigprof(to: libc::pid_t) {

View file

@ -2750,7 +2750,9 @@ class CGWrapMethod(CGAbstractMethod):
unforgeable = CopyUnforgeablePropertiesToInstance(self.descriptor) unforgeable = CopyUnforgeablePropertiesToInstance(self.descriptor)
if self.descriptor.proxy: if self.descriptor.proxy:
create = """ create = """
let handler = RegisterBindings::PROXY_HANDLERS[PrototypeList::Proxies::%(concreteType)s as usize]; let handler: *const libc::c_void =
RegisterBindings::proxy_handlers::%(concreteType)s
.load(std::sync::atomic::Ordering::Acquire);
rooted!(in(*cx) let obj = NewProxyObject( rooted!(in(*cx) let obj = NewProxyObject(
*cx, *cx,
handler, handler,
@ -6744,7 +6746,10 @@ class CGRegisterProxyHandlersMethod(CGAbstractMethod):
def definition_body(self): def definition_body(self):
return CGList([ return CGList([
CGGeneric("PROXY_HANDLERS[Proxies::%s as usize] = Bindings::%s::DefineProxyHandler();" CGGeneric("proxy_handlers::%s.store(\n"
" Bindings::%s::DefineProxyHandler() as *mut _,\n"
" std::sync::atomic::Ordering::Release,\n"
");"
% (desc.name, '::'.join([desc.name + 'Binding'] * 2))) % (desc.name, '::'.join([desc.name + 'Binding'] * 2)))
for desc in self.descriptors for desc in self.descriptors
], "\n") ], "\n")
@ -6753,10 +6758,18 @@ class CGRegisterProxyHandlersMethod(CGAbstractMethod):
class CGRegisterProxyHandlers(CGThing): class CGRegisterProxyHandlers(CGThing):
def __init__(self, config): def __init__(self, config):
descriptors = config.getDescriptors(proxy=True) descriptors = config.getDescriptors(proxy=True)
length = len(descriptors)
self.root = CGList([ self.root = CGList([
CGGeneric("pub static mut PROXY_HANDLERS: [*const libc::c_void; %d] = [0 as *const libc::c_void; %d];" CGGeneric(
% (length, length)), "#[allow(non_upper_case_globals)]\n" +
"pub mod proxy_handlers {\n" +
"".join(
" pub static %s: std::sync::atomic::AtomicPtr<libc::c_void> =\n"
" std::sync::atomic::AtomicPtr::new(std::ptr::null_mut());\n"
% desc.name
for desc in descriptors
) +
"}\n"
),
CGRegisterProxyHandlersMethod(descriptors), CGRegisterProxyHandlersMethod(descriptors),
], "\n") ], "\n")
@ -7606,8 +7619,6 @@ class GlobalGenRoots():
for d in config.getDescriptors(hasInterfaceObject=True) for d in config.getDescriptors(hasInterfaceObject=True)
if d.shouldHaveGetConstructorObjectMethod()]) if d.shouldHaveGetConstructorObjectMethod()])
proxies = [d.name for d in config.getDescriptors(proxy=True)]
return CGList([ return CGList([
CGGeneric(AUTOGENERATED_WARNING_COMMENT), CGGeneric(AUTOGENERATED_WARNING_COMMENT),
CGGeneric("pub const PROTO_OR_IFACE_LENGTH: usize = %d;\n" % (len(protos) + len(constructors))), CGGeneric("pub const PROTO_OR_IFACE_LENGTH: usize = %d;\n" % (len(protos) + len(constructors))),
@ -7624,7 +7635,6 @@ class GlobalGenRoots():
" debug_assert!(proto_id < ID::Last as u16);\n" " debug_assert!(proto_id < ID::Last as u16);\n"
" INTERFACES[proto_id as usize]\n" " INTERFACES[proto_id as usize]\n"
"}\n\n"), "}\n\n"),
CGNonNamespacedEnum('Proxies', proxies, 0, deriving="PartialEq, Copy, Clone"),
]) ])
@staticmethod @staticmethod
@ -7636,8 +7646,6 @@ class GlobalGenRoots():
return CGImports(code, descriptors=[], callbacks=[], dictionaries=[], enums=[], typedefs=[], imports=[ return CGImports(code, descriptors=[], callbacks=[], dictionaries=[], enums=[], typedefs=[], imports=[
'crate::dom::bindings::codegen::Bindings', 'crate::dom::bindings::codegen::Bindings',
'crate::dom::bindings::codegen::PrototypeList::Proxies',
'libc',
], config=config, ignored_warnings=[]) ], config=config, ignored_warnings=[])
@staticmethod @staticmethod

View file

@ -515,8 +515,8 @@ impl EventTarget {
let name = CString::new(format!("on{}", &**ty)).unwrap(); let name = CString::new(format!("on{}", &**ty)).unwrap();
// Step 3.9, subsection ParameterList // Step 3.9, subsection ParameterList
static mut ARG_NAMES: [*const c_char; 1] = [b"event\0" as *const u8 as *const c_char]; const ARG_NAMES: &[*const c_char] = &[b"event\0" as *const u8 as *const c_char];
static mut ERROR_ARG_NAMES: [*const c_char; 5] = [ const ERROR_ARG_NAMES: &[*const c_char] = &[
b"event\0" as *const u8 as *const c_char, b"event\0" as *const u8 as *const c_char,
b"source\0" as *const u8 as *const c_char, b"source\0" as *const u8 as *const c_char,
b"lineno\0" as *const u8 as *const c_char, b"lineno\0" as *const u8 as *const c_char,
@ -524,13 +524,7 @@ impl EventTarget {
b"error\0" as *const u8 as *const c_char, b"error\0" as *const u8 as *const c_char,
]; ];
let is_error = ty == &atom!("error") && self.is::<Window>(); let is_error = ty == &atom!("error") && self.is::<Window>();
let args = unsafe { let args = if is_error { ERROR_ARG_NAMES } else { ARG_NAMES };
if is_error {
&ERROR_ARG_NAMES[..]
} else {
&ARG_NAMES[..]
}
};
let cx = window.get_cx(); let cx = window.get_cx();
let options = unsafe { let options = unsafe {