[Experiment]: Required Send in store traits implementation with a feature flag

[gferon]

See #298

[jrose-signal]

So I'm torn. On the one hand, putting the Context pointer inside the stores for FFI is a tidy solution that avoids having to pass an extra parameter to all the async APIs. On the other, the model of an operation-specific context is still a sensible one even at the Rust level (for a concrete example, the iOS client passes down a database transaction as the context).

If we step back and look at why Context blocks Send…

[R]aw pointers are, strictly speaking, marked as thread-unsafe as more of a lint. Doing anything useful with a raw pointer requires dereferencing it, which is already unsafe. In that sense, one could argue that it would be "fine" for them to be marked as thread safe.

(from https://doc.rust-lang.org/nomicon/send-and-sync.html)

So we could get around this by having Context be a strongly-typed wrapper that implements Send regardless. Or we could make Context into something more general, like a marker trait refining Any + Send so that we can pass Box<dyn Context>. Or we could take your approach of using the stores themselves to carry a per-transaction context (which is always possible). Thoughts?

[lushing]

I would say that the solution presented here feels pretty reasonable. @jrose-signal do you have any particular use case in mind for keeping the context around as a parameter to all these async APIs? Maybe this way deviates from the norms a bit, but it seems like a more pragmatic choice.

[lushing]

Hi @jrose-signal, if you have a particular preference among the potential solutions you presented, do tell, as I am more than willing to help implement whichever solution that can be included upstream.

[jrose-signal]

I feel like iOS's use case of passing a transaction through is a reasonable one, and that we're more likely to want to expose that kind of thing to Java and TypeScript in the future than to have them forever be different. Of course, Rust supports zero-cost wrappers, but I still don't think that leads to a good experience: you'd have to write a wrapper type to carry the extra information and forward the trait methods, and then construct it at the call sites:

message_encrypt(
  message,
  destination,
  SessionStoreWrapper::new(sessionStore, transaction),
  IdentityKeyStoreWrapper::new(identityStore, transaction))

as opposed to

message_encrypt(
  message,
  destination,
  sessionStore,
  identityStore,
  transaction as StoreContext)

So I think I like the idea of Context becoming a marker for Any + Send, and the specific Context used by FFI to be a wrapper type around a pointer with unsafe impl Send. This is okay by this description from the Rustonomicon, not to mention how they'll be used in practice:

However raw pointers are, strictly speaking, marked as thread-unsafe as more of a lint. Doing anything useful with a raw pointer requires dereferencing it, which is already unsafe. In that sense, one could argue that it would be "fine" for them to be marked as thread safe.

[gferon]

So I think I like the idea of Context becoming a marker for Any + Send, and the specific Context used by FFI to be a wrapper type around a pointer with unsafe impl Send.

I also like this approach, I'll try to propose something that goes in that direction.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been closed due to inactivity.