Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Bugs You'll Probably Only Have in Rust

gankro.github.io

Post reply on HN
Print view View original

61–70 of 91 posts

Re: Bugs You'll Probably Only Have in Rust

#61

Post by dbaupp »

grogers wrote: post #60
vitalyd wrote: post #51

Earlier quoted context omitted.

> I may have misunderstood Ralf's bug. Is it really the case that MutexGuard was seen as Sync if T was Send, rather that Sync? Wouldn't that be a bigger problem than just the case of MutexGuard? So T: Sync if &T: Send. MutexGuard internally contains a &Mutex (and Poison, but that's irrelevant here). T was Cell . If you follow the rabbit hole, you'll net out that T was Send, and therefore MutexGuard was Sync.

My confusion (and I suspect others) is about what it means for &T to be Sync. Cell isn't safe to be shared across threads (so isn't Sync) but it is Send if T:Send. But that means &Cell is Sync? You can share a reference to something across threads but not the thing itself? What does that even mean? You could imagine an alternate world where MutexGuard is Send, to allow transfer of ownership of a lock to a different t…

The syntax was a bit confusing: T: Sync means (&T): Send and also (&T): Sync. T being Send or not doesn't affect the threadsafety of &T (Send is about transferring ownership which cannot happen with a &T).

You are correct to be confused about (&Cell) possibly being Sync, because the assumptions that were implied were wrong: when Sync talks about sharing a T, that can be entirely thought of as transferring a &T to another thread (aka Sending the &T). In this sense, sharing a &T between threads (as in, (&T): Sync) is the same as transferring a &&T to another thread, but the inner &T can be copied out so the original &T was also transferred (not just shared) between threads; that is to say, (&T): Sync is 100% equivalent to T: Sync.

Anyway, back to the example here, Cell is not Sync, so neither is &Cell, but M = Mutex> is Sync (this is a major reason Mutex exists in that form: allowing threadsafe shared mutation/manipulation of types that do not automatically support it), and thus &M is Sync too. Since MutexGuard> contains &M, it was thus automatically, incorrectly Sync.

For your second confusion, it is okay for &MutexGuard to be Sync, if MutexGuard itself is. The problem here was MutexGuard was Sync incorrectly in some cases. (MutexGuard is semantically just a fancy wrapper around a &mut T, and so should behave the same as that for traits like Send and Sync.)

Re: Bugs You'll Probably Only Have in Rust

#62

Post by vitalyd »

dbaupp wrote: post #56
vitalyd wrote: post #53

Earlier quoted context omitted.

Somewhat tangential, but what ensures memory visibility in Rust? Say I allocate a struct (heap or stack), and then pass an immutable reference to a function that takes T: Sync. Assume the struct itself is Sync (e.g. bunch of integer fields). What ensures that the other thread sees all writes to this struct prior to the handoff?

It is the responsibility of cross-thread communication abstractions to use the right fencing (if it is touting itself as safe), probably with the various things in std::sync (especially ...::atomics) if it is pure Rust. For instance, spawning a thread, using a channel (std::sync::mpsc) or a mutex all do such things. Just calling a function taking T: Sync doesn't need to do any of this, since that call happens all on…

Ok, that's what I figured - thanks.

That does bring up the question, though, whether it's correct to say that a Sync type doesn't permit data races. In the example I gave above, publishing a Sync struct incorrectly can exhibit data race like symptoms on the receiving thread. So even though the type itself is Sync, that's not enough of a guarantee in the face of "unsafe" publication.

Re: Bugs You'll Probably Only Have in Rust

#63

Post by burntsushi »

vitalyd wrote: post #62
dbaupp wrote: post #56

Earlier quoted context omitted.

It is the responsibility of cross-thread communication abstractions to use the right fencing (if it is touting itself as safe), probably with the various things in std::sync (especially ...::atomics) if it is pure Rust. For instance, spawning a thread, using a channel (std::sync::mpsc) or a mutex all do such things. Just calling a function taking T: Sync doesn't need to do any of this, since that call happens all on…

Ok, that's what I figured - thanks. That does bring up the question, though, whether it's correct to say that a Sync type doesn't permit data races. In the example I gave above, publishing a Sync struct incorrectly can exhibit data race like symptoms on the receiving thread. So even though the type itself is Sync, that's not enough of a guarantee in the face of "unsafe" publication.

It is a guarantee---or else it's a bug (which is the same as every other safe foundation). A type T gets to be `Sync` in one of two ways:

  1. It is "auto derived" when all of its constituent types are Sync.

  2. It is explicitly implemented using `unsafe impl Sync for T {}`. Note the use of the `unsafe` keyword.

Re: Bugs You'll Probably Only Have in Rust

#64

Post by vitalyd »

burntsushi wrote: post #63
vitalyd wrote: post #62

Earlier quoted context omitted.

Ok, that's what I figured - thanks. That does bring up the question, though, whether it's correct to say that a Sync type doesn't permit data races. In the example I gave above, publishing a Sync struct incorrectly can exhibit data race like symptoms on the receiving thread. So even though the type itself is Sync, that's not enough of a guarantee in the face of "unsafe" publication.

It is a guarantee---or else it's a bug (which is the same as every other safe foundation). A type T gets to be `Sync` in one of two ways: 1. It is "auto derived" when all of its constituent types are Sync. 2. It is explicitly implemented using `unsafe impl Sync for T {}`. Note the use of the `unsafe` keyword.

Right, but my question isn't about T itself, but rather how it's published to another thread. The example I gave is of a plain struct with no atomics or any other synchronization types internally. A &T is auto-derived to be Sync. But, if a publisher incorrectly publishes this reference, the other thread may see a partially initialized value.

Re: Bugs You'll Probably Only Have in Rust

#65

Post by vitalyd »

dbaupp wrote: post #47
mcguire wrote: post #44

Wait just a minute. Ralf Jung writes, " This means that the compiler considers a type like MutexGuard to be Sync if all its fields are Sync. " Is that true in general? Is a type thread safe if all its fields are thread safe individually?

Send and Sync are about data races, which lead to memory unsafety, not other forms of thread safety (like dead lock freedom, or maintaining non-unsafe relationships between fields). If there's no unsafe code, then there's no way to have a data race when the individual components are also data race free.

I think it's fair to question a type being auto-derived to be Sync if only individual fields are Sync. It may lead to improper sharing of a reference to this value where threadsafety across fields is needed. That would be a bug, yes, but compiler never made the author pause to consider putting Sync there explicitly (and presumably thinking this through). So that "linting" aspect that's applied to, e.g., *mut T is not present.

Re: Bugs You'll Probably Only Have in Rust

#66

Post by GolDDranks »

vitalyd wrote: post #64
burntsushi wrote: post #63

Earlier quoted context omitted.

It is a guarantee---or else it's a bug (which is the same as every other safe foundation). A type T gets to be `Sync` in one of two ways: 1. It is "auto derived" when all of its constituent types are Sync. 2. It is explicitly implemented using `unsafe impl Sync for T {}`. Note the use of the `unsafe` keyword.

Right, but my question isn't about T itself, but rather how it's published to another thread. The example I gave is of a plain struct with no atomics or any other synchronization types internally. A &T is auto-derived to be Sync. But, if a publisher incorrectly publishes this reference, the other thread may see a partially initialized value.

It's the responsibility of the code that transfers the reference to another thread to ensure that. Deep down, past the abstractions, you can't transfer stuff between threads without using unsafe code. It's the responsibility of this unsafe code to ensure that if the value is visible from another thread, all the writes in the current thread have completed before it's visible. One can do this using memory fences.

Re: Bugs You'll Probably Only Have in Rust

#67

Post by dbaupp »

vitalyd wrote: post #62
dbaupp wrote: post #56

Earlier quoted context omitted.

It is the responsibility of cross-thread communication abstractions to use the right fencing (if it is touting itself as safe), probably with the various things in std::sync (especially ...::atomics) if it is pure Rust. For instance, spawning a thread, using a channel (std::sync::mpsc) or a mutex all do such things. Just calling a function taking T: Sync doesn't need to do any of this, since that call happens all on…

Ok, that's what I figured - thanks. That does bring up the question, though, whether it's correct to say that a Sync type doesn't permit data races. In the example I gave above, publishing a Sync struct incorrectly can exhibit data race like symptoms on the receiving thread. So even though the type itself is Sync, that's not enough of a guarantee in the face of "unsafe" publication.

In safe Rust, sharing a value of any Sync type between threads can't result in data races. Send and Sync provide thread safety guarantees about types that other safe abstractions can rely upon, and fencing correctly is one of the things those abstractions have to do to be safe.

I guess "Sync types don't have data races" is the abbreviated version of "Sync types don't have data races in any safe code, no matter how weird and wonderful". That said, this qualification doesn't seem very interesting to me: something equivalent is required about pretty much any statement about any guarantee in any language with unsafe code or FFI (e.g. in Python, something along the lines of "pointers don't dangle in any code that doesn't use `ctypes`"), and thus is elided in a lot of discussions about programming languages.

If you consider `unsafe` Rust, then failing to fence correctly is just one way to get a data race.

Re: Bugs You'll Probably Only Have in Rust

#68

Post by dbaupp »

vitalyd wrote: post #65
dbaupp wrote: post #47

Earlier quoted context omitted.

Send and Sync are about data races, which lead to memory unsafety, not other forms of thread safety (like dead lock freedom, or maintaining non-unsafe relationships between fields). If there's no unsafe code, then there's no way to have a data race when the individual components are also data race free.

I think it's fair to question a type being auto-derived to be Sync if only individual fields are Sync. It may lead to improper sharing of a reference to this value where threadsafety across fields is needed. That would be a bug, yes, but compiler never made the author pause to consider putting Sync there explicitly (and presumably thinking this through). So that "linting" aspect that's applied to, e.g., *mut T is not…

I'm not entirely sure what you mean.

The compiler does make the author pause: dangerous types like `*mut T and `UnsafeCell` are not Sync, so types containing them are also not automatically Sync.

In any case, Rust only guarantees no memory unsafety (requiring no data races). It tries to help with other things, like cleaning up resources via destructors, but these are "best-effort" rather than guarantees. The only way to get a data race and/or memory unsafety with an automatically implemented Sync is with `unsafe` code, and any `unsafe` code near a data type "infects" it and so means the whole type require great care.

Tooling like asan and tsan and, hopefully, Rust-specific sanitizers and static analyzers will make this easier to get right, but fundamentally as soon as `unsafe` comes into the equation the programmer has to be paranoid. Of course, as the MutexGuard problem indicates, humans getting it right error-prone, which is why the aforementioned tooling and formal proofs---like the one that found that problem---are important, as is building and using appropriate abstractions (e.g. MutexGuard is semantically designed to be a &mut T, so maybe it could indicate this by using PhantomData, or even just storing that directly: this does require manual work, but pushing conventions like that might bridge help the gap to having great `unsafe` tooling in future).

Re: Bugs You'll Probably Only Have in Rust

#69

Post by Manishearth »

vitalyd wrote: post #64
burntsushi wrote: post #63

Earlier quoted context omitted.

It is a guarantee---or else it's a bug (which is the same as every other safe foundation). A type T gets to be `Sync` in one of two ways: 1. It is "auto derived" when all of its constituent types are Sync. 2. It is explicitly implemented using `unsafe impl Sync for T {}`. Note the use of the `unsafe` keyword.

Right, but my question isn't about T itself, but rather how it's published to another thread. The example I gave is of a plain struct with no atomics or any other synchronization types internally. A &T is auto-derived to be Sync. But, if a publisher incorrectly publishes this reference, the other thread may see a partially initialized value.

There are three ways of sharing data across threads.

One is by sharing the data with the thread when it is spawned via a closure. Spawning will fence. No problem there.

The second is to use a good 'ol Sender/Receiver channel pair. These are effectively a shared ring buffer that you can push to and pop from. They also have a fence somewhere.

Finally, you can stick your data into a mutex shared between threads (and let the other thread wait and read it). This will IIRC fence, or do something equivalent.

You can of course build your own ways to do this, but they will need unsafe code to be built (the three APIs above are also built with unsafe code). It is up to you to ensure you handle the fences right when doing this.

The responsibility here is on the publishing mechanism. Most folks use one of the three ways above using primitives from the stdlib depending on the use case.

Re: Bugs You'll Probably Only Have in Rust

#70

Post by jononor »

dbaupp wrote: post #59
Animats wrote: post #55

Earlier quoted context omitted.

That's how you put in a buffer overflow backdoor.

Could you be more specific about what you mean? The relevant (i.e. when it isn't premature optimisation: benchmarks have been run) alternatives here are 1. always risk memory unsafety, fast 2. crash on conditions that would trigger memory unsafety, slow 3. use 1 in release builds and 2 in debug/testing builds. This is effectively having an in-code sanitiser. You can definitely argue that using an actual sanitizer mig…

Another alternative: 4. Provide two codepaths, foo() without unsafe and foo_unsafe() for using the unsafe/fast paths.

Then the consumer/caller can decide on the tradeoffs. Documentation should maybe prefer the safe, a benchmark can show the difference.

Can be used in combination with your current approach of checking in debug builds.

Some test cases should exist to exercise the unsafe checks, and show problems to avoid when using the unsafe variant.

Post reply on HN