libR-sys
libR-sys copied to clipboard
extendr
Make `SEXP` both `Send` and `Sync`
By way of https://github.com/extendr/extendr/issues/659, I think we should make SEXP a wrapper around SEXP, and define Send and Sync for this. It is a pointer, and I see no qualms with making it passable to threads. A higher-level abstraction like Robj should ensure that Send/Sync makes sense, through various means (like Box, Mutex, single_threaded, ownership, etc.).
Yes. But that's for rust types. Types that inherently contain information on whether they are Sync or Send. So you can stipulate with the abstractions mentioned above, what can happen to them.
But SEXP are with no type information. Either we allow them to be used as primitives, that then the higher level api can dress however it see fits, or this wrapping will happen in every other higher end library.
This eases the syntax burden upstream.
Like you wouldn't need to use usize in ownership module.
I can be persuaded that this is a bad idea though, I just want to communicate this opinion.
I might be wrong, but, in my understanding, pointers are not Sync or Send simply because there's no mechanism to prevent data race or access conflict. For example, it's possible you access to an SEXP and it's already GC-ed on another thread.
Like you wouldn't need to use usize in ownership module.
Yeah, I get your point that marking SEXP as Send and Sync is less bad than treating it as usize. But, I think it should be done rather on extendr's side by wrapping it with a new type if needed.
Thank you. I'd like to discuss this some more. I'm still myself not convinced of either direction.
I've this PR, https://github.com/extendr/extendr/pull/666, that shows how wrapping SEXP to have these properties
would look like. This works, but the disadvantages are that every time the R-API is to be used, then you need a sexp.0 / sexp.inner(), etc.
Plus where types are defined, in this case libR-sys, is where things like Send/Sync/Copy/etc. ought to be deifned.
The GC example you mention is interesting. There is only one R-session active. So in a multi-threaded setting,
we'd have either one protection-mechanism, or a proteciton-mechanism per thread. I realise this while writing this
PR https://github.com/extendr/extendr/pull/674, where I'd like to see, how extendr would look like, if we gave up,
on mult-threaded safety. In this case, I'd imagine either sticking to a common (amongst threads) protection mechanism, or a pr. thread protection mechanism, and that if they "share" resources, then they'd basically have said object in both their threads' ownership or just in the common one. Right now, the common protection insures that nothing gets deallocated unless all threads are done with it. And in a pr. thread protection, you'd need the SEXP to be Send, to uphold this as well..
A gc call happening while Rust is running will have no effect on whether an SEXP is still valid or not, because Rust doesn't decide what gets to gc'ed, without the above mentioned influence. This is due to having one common R-session.
But in any of these cases, you'd be sending SEXPs. In the multi-threaded version or not.
So in both implementations, you'd need SEXP to be Send. And in both you'd have to make sure that this make sense for the implementation. And in both you'd benefit from not having redundant sexp.0/sexp.inner() everywhere.
Usually, this is achieved by implementing Deref/DerefMut, but these don't make sense for pointer-things, as none of the R-API wants a reference to a pointer.. So the usual Rust way of dealing with new-type mania fails here.
I'd also emphasize that !Send and !Sync is not agnostic choices. It isn't necessarily "better" to have them be like that, for no good reason.
While that's said, I still don't want this to be forced in, without extensive discussions. The PRs I've made is to explore these ideas.
SIDE NOTE
Just another completely different thing, since SEXPREC is an opaque type, maybe what we should be doing is this: https://doc.rust-lang.org/nomicon/ffi.html#representing-opaque-structs ?