5 ms·

It's interesting, I read this and had the opposite conclusion -- people are still writing racy code using Rust. I mean, it's great that you can write threaded

• brundolf 6y ago
  > What issues we did find were mistakes in the implementations of low-level and explicitly unsafe multithreading abstractions (emphasis mine) The important thing is that the races only happened in unsafe { } blocks. It's well-established that these blocks should basically be treated like C++ in terms of scrutiny, but (I believe) you can still have roughly "absolute confidence" in non-unsafe code. It's true and interesting that unsafe { } blocks deserve sanitization analysis like C++ code does - and maybe this hasn't been discussed enough - but I don't think it's fair to suggest that the enormous language benefits are outweighed by some false sense of confidence. The proportions in that idea just seem way out of whack.
  • benschulz 6y ago
    AFAIU one can create races in safe code using atomics with insufficient ordering. (That notwithstanding, I agree that Rust is a vast improvement over C/++.)
    • steveklabnik 6y ago
      "races" is too general. Safe Rust cannot have data races. It can have race conditions.
      • benschulz 6y ago
        I'm not convinced this is a meaningful distinction in this context. Assuming a developer consciously creates a race which turns out to be a bug, how does it matter whether it was a data race or some other kind of race condition?
        • steveklabnik 6y ago
          Data races are undefined behavior, and race conditions are a logic error. Yes, both are bugs, and both are important to fix. But they have (at least to me) different severities. (Worth noting that data races have a clear, unambiguous definition, and therefore are possible to eliminate through tooling. Race conditions rely on program intent or logic, and are therefore... not really.) Also, I think it matters, generally, that we are truthful about what Rust prevents and does not prevent.
          • brundolf 6y ago
            > Worth noting that data races have a clear, unambiguous definition, and therefore are possible to eliminate through tooling. Race conditions rely on program intent or logic, and are therefore... not really. This is a great point. Usually people only talk about the two categories in terms of vague severity, but this is a hard distinction which has lots of implications for how each can be dealt with.
          • benschulz 6y ago
            > Data races are undefined behavior, and race conditions are a logic error. Yes, both are bugs, and both are important to fix. But they have (at least to me) different severities. Some data races really are benign while some other race conditions can lead to data corruption or security vulnerabilities. Why then should data races be strictly more severe than data races? What am I missing?
          • pjmlp 6y ago
            Any kind of race can lead to data corruption, or being very drastic, possible death if the outcome is related to any computer system with direct influence over human lives like factory automation systems. This is why I think Rust discussions about this subject don't focus enough that the language only validates a very tiny subset of races.
    • volta83 6y ago
      You can also create races in safe Rust by using the file system, no concurrency primitives necessary (just create a new file, close it, and re-open it assuming it's still there..). What safe Rust doesn't have is _data races_.
  • nyanpasu64 6y ago
    The thing is, the linked data race occurred in safe code! Looking at https://bugzilla.mozilla.org/show_bug.cgi?id=1686158 https://bugzilla.mozilla.org/show_bug.cgi?id=1686158, the patch changes a non-atomic read through a &SwCompositeGraphNode and a subsequent non-atomic write through a &mut SwCompositeGraphNode (both safe code). They should not race, and the only reason they do is because there wasn't proper RwLock-style synchronization when handing out &SwCompositeGraphNode and &mut SwCompositeGraphNode. Also I think merely creating an unsynchronized &mut is undefined behavior in of itself (in addition to allowing UB in safe code). If so, even the fixed code is still technically wrong (just not miscompiled or misbehaving in practice). Looking at https://searchfox.org/mozilla-central/rev/ee9dab6aa95f167a34cb178960f7375210a0bba4/gfx/wr/webrender/src/compositor/sw_compositor.rs https://searchfox.org/mozilla-central/rev/ee9dab6aa95f167a34..., SwCompositeGraphNodeRef has "safe" methods (line 286) with unsafe blocks, handing out raw &SwCompositeGraphNode and &mut SwCompositeGraphNode to the interior of an UnsafeCell, without locking or runtime checking. I think SwCompositeGraphNodeRef "caused" the UB because it was invoked by some other caller to create a &mut SwCompositeGraphNode without proper synchronization... And worse yet, SwCompositeGraphNodeRef has Deref and DerefMut implementations that implicitly create & and &mut, unsynchronized, at call sites that merely look like method calls... My theory as to how this happened: - C++ atomics are not mutable through a const *, so the authors used &mut to indicate mutability (even though Rust's &mut has stronger guarantees, according to the Stacked Borrows memory model, "all other pointers to this object are invalidated, except the pointers we reborrowed from"... though Stacked Borrows may change to make async fn and intrusive linked lists sound, I suspect it won't make this code sound). - Maybe they thought "we want to prevent calling &mut SwCompositeGraphNode methods from a &SwCompositeGraphNodeRef". IDK. Semi-related: SwCompositeGraphNodeRef is a #[derive(Clone)] Arc<UnsafeCell<SwCompositeGraphNode>> but !Sync, so only threads that own a SwCompositeGraphNodeRef can create either &SwCompositeGraphNodeRef or &mut SwCompositeGraphNodeRef. And I don't see &SwCompositeGraphNodeRef floating around, so I don't see why access control would matter. (EDIT: The commit introducing the unsound abstraction is https://hg.mozilla.org/integration/autoland/rev/8d47e408c578 https://hg.mozilla.org/integration/autoland/rev/8d47e408c578, filed under the bug at https://bugzilla.mozilla.org/show_bug.cgi?id=1670328 https://bugzilla.mozilla.org/show_bug.cgi?id=1670328.)
• agumonkey 6y ago
  hopefully it will lead to a new borrow checker logic
• fluidcruft 6y ago
  Is that "sprinkle atomics" stuff in bindings/interfaces for non-Rust external code? I'm not particularly surprised that you would have to wrap atomics around externalities and not doing that would be a source of errors if the external code has race conditions.
  • evmar 6y ago
    It's not, I linked the patch. (I don't fully understand the patch so I'm happy to be corrected here, but it looks like it was an ordinary "use atomics incorrectly" bug.)
    • tialaramex 6y ago
      It's unsafe code (see the "unsafe" keyword?). So, Rust relies on the people who wrote it to do so correctly, and in this case they mistakenly assumed they could get away without one variable being atomic. The patch corrects that, making it atomic. So Rust did exactly what we wanted here, the problem code was narrowed down to the code that was explicitly unsafe and needing caution. Humans remained fallible, and the ThreadSanitizer spotted their mistake.
• dan-robertson 6y ago
  Another fallacy to keep in mind is that two things not being perfect does not make them equal failures.
• est31 6y ago
  You are correct. The "no data races" slogan of Rust is accurate but you need to mention the way it needs to be understood. First, data races are only a subset of all race conditions. Second, like the other safety guarantees, the statement only applies to safe Rust. Once you use unsafe Rust or any non-Rust language (and both usually happens in any non trivial program), this guarantee stops being a guarantee. But it doesn't mean it vanishes into nothingness. It instead becomes a quantitative statement: it's much easier to avoid data races in Rust than in unsafe alternatives. I think when you build Rust projects that require reliability, it's foolish to lean back and believe that marketing slogan and not look for concurrency bugs. However, once you found them, it's way easier to fix them, at least when your unsafety is well encapsulated like how it's recommended. I think the big challenge for Rust in this context is to improve its tooling so that using it is comparable or even easier than the C++ tooling. This blog post is proof that such tools should be ran otherwise you are missing out on important bugs.
• coliveira 6y ago
  I think the mistaken idea that Rust allows "bug free" multithreaded programs will lead to a new generation of difficult to maintain multithreaded software... The lesson of multithreading should be that you need to use less of it and only when necessary, not that you should write more. It is similar to what we got when Java was the darling against C++: you now have lots of Java programs that don't have the same problems as the C++ version, but still are difficult to understand, leading to the well known FactoryFactory syndrome.
  • mplanchard 6y ago
    I honestly don’t see “less concurrency” as a viable route for modern software. We’re adding cores at a much faster rate than we’re improving clock speeds. Anything that makes concurrency easier is a win in my book.
    • coliveira 6y ago
      There are several ways to increase concurrency: low level and high level. UNIX, for example, provided high level tools for concurrency (processes). You can also use message passing if you want. It is possible to write more concurrent software using safer primitives, instead of relying in low level techniques such as Rust/C++ multithreading.
      • tsimionescu 6y ago
        I don't think it makes sense to call processes either higher or lower level than threads. Splitting your program into multiple processes often comes with many other drawbacks than spitting it into multiple threads. You can even still have data races if you use files or mmap to communicate between processes (you can also get something similar even with pipes/sockets for complex chunked data structures). Even worse, there are very few, if any, tools that could help you analyze a multiprocess program for any kind of race conditions.
        • pjmlp 6y ago
          When processes crash or corrupt memory they don't bring all the application down with them like threads, hence why security focused architectures have moved back to processes after the thread adoption hype.
• stjohnswarts 6y ago
  They literally found tons of new races conditions in c++ and 2 in the rust code with their tool. And you want to act as if that little factoid makes rust useless somehow. Anyone who thinks a language is flawless is not thinking correctly and that includes rust. However it does a much better job at this than C/C++ yet somehow you twisted that into declaring rust as useless.
  • evmar 6y ago
    I'm not sure how you got from "Rust gives you more confidence but not absolute confidence and you still need the normal stable of runtime detection tooling" to "useless".
    • frenchy 6y ago
      It's probably because "Rust gives you absolute confidence" is a strawman, I don't think anyone was arguing that, and often people's first reaction to a strawman is to construct a contrary strawman.
• oconnor663 6y ago
  > you still need the normal stable of runtime detection tooling I hate that you're getting so many downvotes, and I wish people wouldn't do that...but I still want to disagree slightly :) I think it's interesting to note that the meaning of "you" changes between the two cases. If I write safe code on top of parking_lot, you could argue that I don't really need to do runtime race detection. Rather, someone in the ecosystem needs to do it, but once it's been done, the shared code is improved for everyone. If my code is all safe, I can benefit from these global improvements over time, and I can be confident that I haven't introduced any new races myself.
• pcwalton 6y ago
  "No true Scotsman" is a fallacy because "true Scotsman" doesn't have a clear definition and can mean anything. But "unsafe" has a very precise meaning in Rust. Safe Rust didn't have any data races, which is exactly what we would expect.