help: capnp::message::Reader does not implement the Sync trait

[gjcarneiro]

Hi, I am getting into an error trying to share capnp messages between threads.
Essentially, my data structure is HashMap<String, capnp::message::Reader<capnp::serialize::OwnedSegments>>. I am trying to protect it with Arc and RwLock (Arc::new(RwLock::new(...))), but still rust doesn't allow it.

It seems to be an error specific to capnp message reader. Other data types work fine. I don't understand why capnp::message::Reader doesn't implement Sync? AFAIK, a reader is just a view into a memory blob, with some methods to extract data. With the RwLock, it should be possible to read this data from multiple threads, and write from one.

   --> src/main.rs:419:5
    |
419 |     thread::spawn(move || {
    |     ^^^^^^^^^^^^^ `std::cell::Cell<u64>` cannot be shared between threads safely
    |
    = help: within `(std::string::String, capnp::message::Reader<capnp::serialize::OwnedSegments>)`, the trait `std::marker::Sync` is not implemented for `std::cell::Cell<u64>`
    = note: required because it appears within the type `capnp::private::arena::ReadLimiter`
    = note: required because it appears within the type `capnp::private::arena::ReaderArenaImpl<capnp::serialize::OwnedSegments>`
    = note: required because it appears within the type `capnp::message::Reader<capnp::serialize::OwnedSegments>`
    = note: required because it appears within the type `(std::string::String, capnp::message::Reader<capnp::serialize::OwnedSegments>)`
    = note: required because of the requirements on the impl of `std::marker::Sync` for `hashbrown::raw::RawTable<(std::string::String, capnp::message::Reader<capnp::serialize::OwnedSegments>)>`
    = note: required because it appears within the type `hashbrown::map::HashMap<std::string::String, capnp::message::Reader<capnp::serialize::OwnedSegments>, std::collections::hash_map::RandomState>`
    = note: required because it appears within the type `std::collections::HashMap<std::string::String, capnp::message::Reader<capnp::serialize::OwnedSegments>>`
    = note: required because it appears within the type `(std::string::String, std::collections::HashMap<std::string::String, capnp::message::Reader<capnp::serialize::OwnedSegments>>)`
    = note: required because of the requirements on the impl of `std::marker::Sync` for `hashbrown::raw::RawTable<(std::string::String, std::collections::HashMap<std::string::String, capnp::message::Reader<capnp::serialize::OwnedSegments>>)>`
    = note: required because it appears within the type `hashbrown::map::HashMap<std::string::String, std::collections::HashMap<std::string::String, capnp::message::Reader<capnp::serialize::OwnedSegments>>, 

[...]

[gjcarneiro]

OK, I figured it out: now I call messag_reader.into_segments() and store capnp::serialize::OwnedSegments in the hash map, instead of storing capnp::message::Reader<capnp::serialize::OwnedSegments>.

[dwrensha]

The std::cell::Cell<u64> is for the read limiter, which enforces the message traversal limit.
Maybe we should make that an AtomicU64 so that it is Sync? I don't think there should be much of a performance difference, especially if we use Relaxed memory ordering.

store capnp::serialize::OwnedSegments in the hash map

Hm... probably then you're going to want to have an impl <'a> ReaderSegments for &'a OwnedSegments.

Maybe we should add a blanket impl: impl <'a, T> ReaderSegments for &'a T where T: ReaderSegments { ...}.

(If we don't add something like that on the capnp end, then you'll need to define a newtype to get it to work.)

[gjcarneiro]

I... don't understand most what you say, I too new to Rust, sorry!

Calling capnp::message::Reader.into_segments() and then storing capnp::serialize::OwnedSegments in the hash table certainly seems to work, but is not very ergonomic: every time I need to read the message while being stored in the HashMap I have to remove it from the HashMap first, read it (using capnp::message::Reader::new(segments, options)), then insert it back into the HashMap.

OTOH, storing capnp::message::Reader in the HashMap allows me to read the message fields while stored in the hashmap, but I don't see any way to just read the bytes of the message (I will need to hand off some of the stored messages to Python): I only see capnp::message::Reader.into_segments(), but this moves the bytes out of the capnp::message::Reader and consequently cannot be called while the message::Reader is still being stored in the HashMap.

But, like I said, still learning Rust, might have missed some things...

[dwrensha]

I added the blanket impl: 92bf1d8. This means that you should be able to immutably borrow a &OwnedSegments from your hashmap and pass it to capnp::message::Reader::new(). Does that work for you?

[gjcarneiro]

Yes! Definitely helps, thank you! 👍

[dwrensha]

I've released 0.14, which has atomic read limiting. https://dwrensha.github.io/capnproto-rust/2020/12/19/atomic-read-limiting.html