crossbeam-rs / crossbeam

Tools for concurrent programming in Rust
Apache License 2.0
7.43k stars 470 forks source link

Request: Send + Sync version of a Handle #207

Open joshlf opened 6 years ago

joshlf commented 6 years ago

For data structures that want to be concurrent and only expose handles, the fact that crossbeam Handles are neither Send nor Sync is an issue. For example, in elfmalloc, we'd like to have allocator handles which are, at the very least, Sync so that you can share them between threads by calling clone from a new thread, but the fact that Handles are not Sync has prevented us from doing that without resorting to unsafe code.

The problem comes down to the fact that Handles use immutable methods in thread-unsafe ways. You could imagine a handle where the pin and is_pinned methods were mutable, leaving clone as the only immutable method, which would be implemented as self.collector().register(). In fact, in order to sidestep this issue, that's exactly what I'm doing right now for elfmalloc:

/// A `Sync` handle on an epoch GC instance.
/// 
/// A `SyncHandle` is a handle on an epoch GC instance which is `Sync`. It is
/// meant to make it easier to work correctly with `Handle`s in a thread-safe
/// manner.
/// 
/// `Handles` are not `Sync` because they keep thread-unsafe references to a
/// `Local` under the hood. `SyncHandle` sidesteps that issue by ensuring that
/// the only immutable method is clone, which is careful not to use the `Handle`
/// stored internally - only the `Arc<Collector>`, which *is* thread-safe.
pub struct SyncHandle {
    handle: Handle,
    collector: Arc<Collector>,
}

impl SyncHandle {
    pub fn new() -> SyncHandle {
        let collector = Arc::new(Collector::new());
        SyncHandle {
            handle: collector.register(),
            collector,
        }
    }

    pub fn pin(&mut self) -> Guard {
        self.handle.pin()
    }
}

impl Clone for SyncHandle {
    fn clone(&self) -> SyncHandle {
        let handle = self.collector.register();
        SyncHandle {
            handle,
            collector: self.collector.clone(),
        }
    }
}

unsafe impl Sync for SyncHandle {}

It would also be nice if such a handle could be Send, although I'm not sure how that would be done with the current Local-based design.

joshlf commented 6 years ago

Oh and if you want to put something in a lazy_static!, it has to be both Send and Sync.

Thomasdezeeuw commented 6 years ago

Can't you use a Collector? It is just an Arc<Global>: https://github.com/crossbeam-rs/crossbeam-epoch/blob/master/src/collector.rs#L23-L25. Handle itself is threadlocal by design: https://github.com/crossbeam-rs/crossbeam-epoch/blob/master/src/collector.rs#L64-L65.

joshlf commented 6 years ago

You can, but that has performance implications - every single time you want to use the GC, you have to register a new handle, which is significantly more expensive than pinning because it involves an allocation and an atomic list insertion.

jeehoonkang commented 6 years ago

In #71, we discussed that Handle's clone() is not that practically useful. It seems it's relevant to this issue. At the end of the day, we need a good interface of Handle (or SyncHandle), which accordingly implements clone() or not.