Is it possible to avoid Sync?

[johanneskoester]

Hi Adam, this is nice work! I have the following code:

// read from BCF
    let mut records = bcf.records().map(|record| Site::new(record.ok().expect("Error reading BCF")));
    let mut candidates = Vec::with_capacity(buffer_size);

    // calculate posterior probabilities in parallel
    cue::pipeline(
        "calling",
        threads,
        records,
        |site| {
            let likelihoods = site.genotype_likelihoods().ok().expect("Error reading genotype likelihoods.");
            let posterior = query.call(&likelihoods);
            (site, posterior)
        },
        |(site, prob)| {
            if prob <= max_prob {
                candidates.push((site, prob));
            }
        }
    );

Unfortunately, it does not compile, because the Site objects (which contain a C struct) only implement Send, not Sync. Here is the error:

src/call/mod.rs:51:5: 51:18 error: the trait bound `*mut htslib::htslib::vcf::Struct_Unnamed40: std::marker::Sync` is not satisfied [E0277]
src/call/mod.rs:51     cue::pipeline(
                       ^~~~~~~~~~~~~
src/call/mod.rs:51:5: 51:18 help: run `rustc --explain E0277` to see a detailed explanation
src/call/mod.rs:51:5: 51:18 note: `*mut htslib::htslib::vcf::Struct_Unnamed40` cannot be shared between threads safely
src/call/mod.rs:51:5: 51:18 note: required because it appears within the type `htslib::bcf::Record`
src/call/mod.rs:51:5: 51:18 note: required because it appears within the type `call::site::Site`
src/call/mod.rs:51:5: 51:18 note: required because it appears within the type `(call::site::Site, f64)`
src/call/mod.rs:51:5: 51:18 note: required because of the requirements on the impl of `std::marker::Sync` for `std::ptr::Unique<(call::site::Site, f64)>`
src/call/mod.rs:51:5: 51:18 note: required because it appears within the type `alloc::raw_vec::RawVec<(call::site::Site, f64)>`
src/call/mod.rs:51:5: 51:18 note: required because it appears within the type `std::vec::Vec<(call::site::Site, f64)>`
src/call/mod.rs:51:5: 51:18 note: required because it appears within the type `&mut std::vec::Vec<(call::site::Site, f64)>`
src/call/mod.rs:51:5: 51:18 note: required because it appears within the type `[closure@src/call/mod.rs:60:9: 64:10 max_prob:&f64, candidates:&mut std::vec::Vec<(call::site::Site, f64)>]`
src/call/mod.rs:51:5: 51:18 note: required by `cue::pipeline`

Do you see a solution for me? Or can the Sync constraint be removed perhaps?

[anp]

Thanks! Exciting to see someone trying it out :).

I don't think the constraint can be removed, but I may be wrong. The work and result queues must implement Send + Sync to be able to be referenced in the worker/joiner threads. The queue implementations I'm using only implement Send + Sync when their contained types also implement Send + Sync (which is important). Which is why that generic bound is enforced on the pipeline function.

So, a solution. Send and Sync can be implemented for any arbitrary type as an unsafe impl (AFAIK, based on the nomicon). The trick IIUC is to manually ensure that they can be used safely with aliasable references (i.e. don't leak the raw pointers with pub fields or as return values, only mutate values behind raw pointers in methods which accept self/&mut self), and then implement Sync as a marker trait. For example, nearly all of the standard library's collections have implementations of the marker traits, despite their use of raw pointers and other unsafe-isms. Granted, you can implement Sync on any ol' type you want, whether it's safe or not..

Is it possible to audit the Site type and its raw pointer fields for any potential thread safety issues and to then write unsafe impl Sync for Site {}?

[johanneskoester]

Thanks for the clarification about sync! I am using rust-htslib here. Would be great if we basically have sync support for free there. I think this is very likely, because we indeed wrap everything into safe methods. I will have a look.

[johanneskoester]

Thanks, I've done that and it seems to work!

[anp]

Glad to hear it!