About the Sync requirement

[diwic]

So, I'm being asked that my crate's error type, dbus::Error should implement Sync because that makes integration with the failure crate easier.

However, I cannot do that, because dbus::Error is an opaque FFI struct, and the C library does not guarantee to uphold Rust's Sync requirement.

I suppose there are reasons for the Sync requirement but can you please reconsider them? Reading an Error from more than one thread at a time doesn't seem like something you want to do very often, and the requirement is causing trouble here.

[Ixrec]

As I understand it, requiring Send+Sync is very much by design and is meant to be a major improvement over std::error::Error, because it makes failure errors and libraries using them compatible with multi-threaded code, and because the vast majority of error types are Send+Sync anyway.

https://github.com/rust-lang-nursery/failure/issues/173 asked basically the same question about Send, to which the official response was:

This sounds like it might just not be a good use case for failure.

What if you had one type that was cheap but does not implement Fail, and it implements a conversion into another type that is Send + Sync ... ?

[diwic]

I can understand Send, but Sync both cuts more errors out, and seems less useful to me.

What if you had one type that was cheap but does not implement Fail, and it implements a conversion into another type that is Send + Sync ... ?

That is already the case in dbus actually.

[withoutboats]

However, I cannot do that, because dbus::Error is an opaque FFI struct, and the C library does not guarantee to uphold Rust's Sync requirement.

Can you clarify how you are able to uphold Send but not Sync?

The Sync bound was added because of a concrete use case for it - chucklefish call Rust from lua, which expects as a part of its runtime for errors to be readable from multiple threads.

[diwic]

Can you clarify how you are able to uphold Send but not Sync?

It's from the libdbus documentation, which states: "Most other objects, however, do not have locks - they can only be used from a single thread at a time, unless you lock them yourself."

It's always a bit shaky to translate C thread safety statements to Rust, but "a single thread at a time" does sound like the Send requirement is fulfilled.

[withoutboats]

@diwic I have to say that seems like very shakey grounds to make decisions that could result in potentially undefined behavior! That quote doesn't even say anything in particular about the guarantees that other APIs provide:

• Saying that they can't be used from multiple threads doesn't mean they can be moved between threads (every type can be used from 1 thread a time, even if it doesn't implement either Send or Sync)
• The word "used" there is very important - C doesn't have a distinction between & and &mut views.

The real question is whether or not they use any kind of interior mutability - if all mutating APIs are exposed as &mut self, your type definitely implements both Send and Sync. This is where the meaning of the word "used" becomes very important: as long as no &self methods mutate any state (including global state), there's no potential for data races.

I would recommend doing this: only using &self for methods that you know don't mutate state without an internal lock, and implement Send and Sync for all types.

[diwic]

I would recommend doing this: only using &self for methods that you know don't mutate state without an internal lock, and implement Send and Sync for all types.

For std::error::Error, the methods that need to take &self are description (and cause where applicable), as well as trait impls for Debug and Display. If either of these need to call into FFI internally, and you cannot guarantee that interior mutability does not happen at that point, you cannot implement Sync.

For me "a single thread at a time, unless you lock them yourself" implies that you can put a lock around your object and then use it from multiple threads. This would not be possible unless the type were in fact Send.

That said, I've taken another look at dbus::Error and it looks like we could get away without calling into FFI at all, thus guaranteeing that no internal state change happens, and therefore be okay with taking &self even with Sync.

[diwic]

That said, I've taken another look at dbus::Error and it looks like we could get away without calling into FFI at all, thus guaranteeing that no internal state change happens, and therefore be okay with taking &self even with Sync.

This is in fact the case, so for my own part it's now fixed.

Meanwhile, at least one instance of someone adding unsafe impl Sync for MyError just to be Failure-compatible (without considering whether or not this is sound!) has been found in the wild. :-/

[gnzlbg]

Another data point is that currently it is hard to use failure within procedural macros. @dtolnay mentioned about why syn errors are not Sync:

Within a procedural macro you should never do this because it loses the ability to emit the error to the compiler in a way that points to the correct range of input tokens like a native compiler error.

The workaround appears straightforward failure::err_msg(err.to_string()) though.

[dtolnay]

Specifically, the compiler's Span which represents token location information is an index into a threadlocal interner to keep the size small so accessing a span from a different thread is not meaningful.

We have the workaround @gnzlbg mentioned so I would keep the Sync requirement in failure.

[nhynes]

We have the workaround @gnzlbg mentioned so I would keep the Sync requirement in failure.

It's not so great if you want to store the span for reporting diagnostics, though.