Is Tendril really Sync?

[SimonSapin]

#17 introduces optional atomic refcounting and adds:

unsafe impl<F, A> Send for Tendril<F, A> where F: fmt::Format, A: Atomicity + Sync { }
unsafe impl<F, A> Sync for Tendril<F, A> where F: fmt::Format, A: Atomicity + Sync { }

(A: Sync iff it’s atomic.)

I think the first impl is OK, but I’m less convinced about the second one. @chris-morgan, what do you think?

[bluss]

No, make_buf_shared (private) uses unguarded Cell::get/set, and it is called from pub unsafe fn unsafe_subtendril(&self,

I don't see anything inside safe method pub fn try_subtendril(&self that would make it not racy to call unsafe_subtendril from there.

[SimonSapin]

Thanks for looking into this. I believe the point of #17 was mostly to make Tendril<F, Atomic>: Send, and that much is ok because the only mutable thing shared between tendrils that share a buffer is the reference count. Does this sound right?

[bluss]

The same Tendril reference can be accessed from two different threads (because Tendril is Sync), and then using .make_buf_shared() is racy.

I think the burden of proof should be on the user of Cell to prove that it is correct to use across threads. If we're not sure, tendril is not Sync.

[SimonSapin]

Right, I agree that Tendril: Sync can very probably be used to form a data race. So we should remove that impl.

However, I think we can keep the Send impl. With Send but not Sync, the only way to have something shared across threads is to clone a tendril and send it to another thread. This leaves two tendrils (each with its independent Cell) that share a byte buffer and its header. The only mutable thing in Header<_> is the refcount, which protects against data races by being AtomicUsize in Header<Atomic>.

Does Tendril<F, Atomic>: Send (but not Sync) sound ok to you?

[bluss]

Ah, thanks. Yes that sounds good. There's a lot of code to read and I can't say I reviewed all of it, though.