~mht/cmr

5b585576fd05856862c7b665eecadc83bccfc1dd — Martin Hafskjold Thoresen 3 years ago 7d2b824
Add fences around the allocator wrappers

I don't see why this should be necessary, but threads get stuck deep inside
jemalloc while they are being signaled, which is exactly what the alloc wrappers
and lock things are supposed to prevent.

The dissappearence of the deadlock isn't 100% confirmed yet, but I'm commitin in
order to run it on the server (where it was more prevalent than on my laptop).
4 files changed, 21 insertions(+), 5 deletions(-)

M Cargo.toml
M benchmarks/Cargo.toml
M src/alloc.rs
M src/lib.rs
M Cargo.toml => Cargo.toml +1 -1
@@ 16,8 16,8 @@ jemalloc-free-hack = { path = "./jemalloc-free-hack" }
git = "https://github.com/martinhath/libc"

[profile.release]
debug = true
lto = true
debug = true

[features]
sanitize = []

M benchmarks/Cargo.toml => benchmarks/Cargo.toml +0 -1
@@ 10,7 10,6 @@ trench = "0.3"
cmr = { path = "../" }
cmr-data-structures = { path = "../data-structures/" }


[features]
noop = ["cmr/noop", "cmr-data-structures/noop"]
disable-reclamation = ["cmr/disable-reclamation"]

M src/alloc.rs => src/alloc.rs +18 -3
@@ 252,6 252,10 @@ pub fn without_reclamation<R, F: FnOnce() -> R>(f: F) -> R {

use std::sync::atomic::AtomicPtr;
thread_local! {
    /// Every thread stores a pointer to a static bool, which is in the array
    /// `cmr::THREAD_BOOLS`; This is an array that contains a single bool for
    /// each thread in the system, which signals whether that thread is
    /// currently allocating or not.
    pub(crate) static SOME_LOCK_MARKER: AtomicPtr<AtomicBool> =
        AtomicPtr::new(::std::ptr::null_mut());
}


@@ 285,14 289,21 @@ impl Asd {
        SOME_LOCK_MARKER.with(|b| {
            let ptr = b.load(SeqCst);
            if ptr.is_null() {
                // `start_alloc` is called by all threads, no matter if they're using CMR or not.
                // If `ptr` is `null`, it means they were never registered by `cmr::thread_activate`,
                // so it will never be signaled, and hence they don't need to tell anyone they are
                // allocating.
                return None;
            }
            let b: &AtomicBool = unsafe { &*ptr };

            let b = unsafe { &*ptr };
            // Mark that we want to allocate
            b.store(true, SeqCst);
            if self.lock.load(SeqCst) {
            // If some other thread has decided to lock allocation, reset our flag
            while self.lock.load(SeqCst) {
                b.store(false, SeqCst);
                while self.lock.load(SeqCst) {}
                b.store(true, SeqCst);
            }
            Some(b)
        })


@@ 302,7 313,7 @@ impl Asd {
        b.store(false, SeqCst);
    }

    /// Return `true` if we already had the lock.
    /// Return `None` if we already had the lock.
    pub(crate) fn lock(&self) -> Option<AsdGuard> {
        if self.owner.load(SeqCst) == super::thread_id() {
            return None;


@@ 330,7 341,9 @@ pub(crate) struct WrappedAllocator;
unsafe impl GlobalAlloc for WrappedAllocator {
    unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
        if let Some(b) = SOME_LOCK.start_alloc() {
            ::std::sync::atomic::compiler_fence(SeqCst);
            let ret = Jemalloc.alloc(layout);
            ::std::sync::atomic::compiler_fence(SeqCst);
            SOME_LOCK.end_alloc(b);
            return ret;
        }


@@ 349,7 362,9 @@ pub(crate) struct WrappedSystemAllocator;
unsafe impl GlobalAlloc for WrappedSystemAllocator {
    unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
        if let Some(b) = SOME_LOCK.start_alloc() {
            ::std::sync::atomic::compiler_fence(SeqCst);
            let ret = System.alloc(layout);
            ::std::sync::atomic::compiler_fence(SeqCst);
            SOME_LOCK.end_alloc(b);
            return ret;
        }

M src/lib.rs => src/lib.rs +2 -0
@@ 15,9 15,11 @@

extern crate alloc_system;
extern crate jemallocator;

#[cfg(not(feature = "system-allocator"))]
#[global_allocator]
static ALLOC: alloc::WrappedAllocator = alloc::WrappedAllocator;

#[cfg(feature = "system-allocator")]
#[global_allocator]
static ALLOC: alloc::WrappedSystemAllocator = alloc::WrappedSystemAllocator;