Facebook Mandates Sanitizers as Hidden C++ Traps Trigger Site Outages

CppCon 2017: Louis Brandy “Curiously Recurring C++ Bugs at Facebook” — CppCon

CppCon

Developer education and code reviews fail to stop recurring outages because C++ grammar quirks and copy paranoia silently introduce dangling references and phantom locks, making automated AST linters and AddressSanitizer the only effective defense.

AI-assisted summary of CppCon 2017: Louis Brandy “Curiously Recurring C++ Bugs at Facebook” — original source. Check the original for full context.

Source published:

Key ideas

  • AddressSanitizer (-fsanitize=address) remains the single highest-impact tool for eradicating use-after-free and dangling reference bugs, outperforming documentation and educational efforts. (4:35)
  • Accidental element insertion via std::map::operator[] on missing keys caused multiple major site-wide outages at Facebook by inserting zero-value timeouts that permanently blocked threads. (9:29)
  • Ad-hoc RCU implementations using std::shared_ptr frequently trigger race conditions because the shared_ptr struct itself is not thread-safe for unsynchronized concurrent read/write access. (25:17)
  • Grammar disambiguation rules parse unnamed lock declarations like std::unique_lock<std::mutex>(mtx) as local variable declarations shadowing the mutex, resulting in unlocked critical sections. (31:42)

Detailed notes

## Background This talk by Louis Brandy, Engineering Director at Facebook, was published on YouTube by CppCon on November 2, 2017. As described in the presentation notes, Brandy shares recurring, high-impact C++ bug patterns observed as Facebook's codebase scaled by orders of magnitude, exploring subtle pitfalls in seemingly simple code and advocating for better tooling, libraries, and language practices to eradicate them.

## TL;DR Louis Brandy examines several notorious and recurring bug categories that plague large-scale C++ codebases despite extensive code reviews and education. He highlights subtle pitfalls such as unintended insertions from `std::map::operator[]`, dangling references created by passing temporaries into fallback lookup helpers, and widespread thread-safety misunderstandings surrounding `std::shared_ptr`. Brandy also dissects insidious grammatical traps like phantom lock declarations caused by default-constructible RAII guards shadowing existing mutexes. Ultimately, he argues that lectures and manual reviews fail to prevent these traps, advocating instead for runtime instrumentation like AddressSanitizer, custom AST linters, and safe API design.

## Atmosphere & style The presentation is candid, fast-paced, and darkly humorous, capturing the collective exhaustion and camaraderie of engineers who have spent weeks tracking down one-character defects. Brandy engages the audience directly with playful quizzes and self-deprecating anecdotes from Facebook site incident reviews, even teasing C++ standards committee members in attendance. Watching the session feels like an insider post-mortem mixed with a pragmatic tooling clinic, striking a balance between existential dread over C++'s parsing quirks and optimistic resolve for automated compiler sanitizers.

## Bulletin - [04:35](https://www.youtube.com/watch?v=lkgszkPnV8g&t=275s) AddressSanitizer (-fsanitize=address) remains the single highest-impact tool for eradicating use-after-free and dangling reference bugs, outperforming documentation and educational efforts. - [09:29](https://www.youtube.com/watch?v=lkgszkPnV8g&t=569s) Accidental element insertion via std::map::operator[] on missing keys caused multiple major site-wide outages at Facebook by inserting zero-value timeouts that permanently blocked threads. - [25:17](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1517s) Ad-hoc RCU implementations using std::shared_ptr frequently trigger race conditions because the shared_ptr struct itself is not thread-safe for unsynchronized concurrent read/write access. - [31:42](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1902s) Grammar disambiguation rules parse unnamed lock declarations like std::unique_lock<std::mutex>(mtx) as local variable declarations shadowing the mutex, resulting in unlocked critical sections.

## Key takeaways - [00:00](https://www.youtube.com/watch?v=lkgszkPnV8g&t=0s) Facebook's core infrastructure heavily relies on C++, which forms the foundational runtime for services, libraries like Folly, and the HipHop Virtual Machine (HHVM). - [04:35](https://www.youtube.com/watch?v=lkgszkPnV8g&t=275s) Out-of-bounds access via vector subscripts and raw pointer dereferences is the most frequent source of bugs, best mitigated by AddressSanitizer and range-based abstractions. - [09:29](https://www.youtube.com/watch?v=lkgszkPnV8g&t=569s) The subscript operator on std::map default-constructs missing keys silently, leading developers to bypass const-correctness compiler errors by simply removing 'const'. - [09:29](https://www.youtube.com/watch?v=lkgszkPnV8g&t=569s) Default insertions into configuration maps led to severe outages when missing 'timeout' keys defaulted to zero, signaling infinite timeouts in network sockets. - [12:35](https://www.youtube.com/watch?v=lkgszkPnV8g&t=755s) Returning references in fallback lookup functions like findWithDefault risks severe use-after-free bugs when callers pass temporary fallback arguments. - [17:08](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1028s) Facebook eradicated the misuse of volatile as a multithreaded synchronization primitive by implementing aggressive, lecturing linter rules and providing proper atomic abstractions. - [22:38](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1358s) std::shared_ptr consists of three distinct parts with different concurrency guarantees: an unsynchronized payload object, an atomic control block, and an unsynchronized pointer pair struct. - [25:17](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1517s) Ad-hoc Read-Copy-Update patterns that swap a global shared_ptr without a mutex cause data races on the pointer struct, leading to torn reads and double frees. - [28:43](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1723s) Immediately dereferencing a temporary shared_ptr (e.g., *getSharedThing()) destroys the managed instance at the end of the full expression, leaving an unprotected dangling reference. - [29:10](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1750s) Due to C++ disambiguation rules, std::string(foo); is parsed as a variable declaration rather than a temporary object construction. - [31:42](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1902s) Writing std::unique_lock<std::mutex>(my_mutex); does not lock anything; it declares an unlocked local lock guard named 'my_mutex' that shadows the class member. - [37:14](https://www.youtube.com/watch?v=lkgszkPnV8g&t=2234s) The phantom lock guard defect requires both RAII semantics and a default constructor, making types like std::unique_lock and custom lock implementations particularly vulnerable. - [39:12](https://www.youtube.com/watch?v=lkgszkPnV8g&t=2352s) Targeted compiler flags like -Wshadow-compatible-local miss lock-shadowing bugs because the lock and mutex have incompatible types, necessitating full -Wshadow or custom AST linters. - [42:03](https://www.youtube.com/watch?v=lkgszkPnV8g&t=2523s) Automated, immediate developer feedback from compilers, linters, and runtime sanitizers is vastly more effective at eliminating bug patterns than retrospective tech talks and documentation.

## Detailed walkthrough ### [00:00](https://www.youtube.com/watch?v=lkgszkPnV8g&t=0s) Introduction and C++ at Facebook Scale

Louis Brandy introduces himself as an engineering director at Facebook who works on the core C++ group, noting that despite people coming in out of the rain after lunch, he is there to talk about bugs. He frequently encounters the misconception that Facebook only writes PHP, but he clarifies that Facebook infrastructure has actually been built on C++ from the very beginning. It stands as the company's second most used language right behind their PHP/Hack codebase—and even Hack runs on HipHop Virtual Machine (HHVM), an enormous C++ process developed in their repository. Across the rest of the infrastructure, Python holds third place, followed by Java, and then a long tail of other languages.

```mermaid flowchart TD accTitle: Facebook Core Languages accDescr: Hierarchy of major programming languages powering Facebook infrastructure.

subgraph CoreInfra["Facebook Infrastructure"] Hack["PHP / Hack Codebase"] -->|"runs on"| HHVM["HHVM (C++ Process)"] CPP["C++ (Core Infrastructure)"] Py["Python"] Java["Java"] Tail["Long tail of other languages"] end ```

The core C++ group at Facebook is logically split into two main sections: - The **tools side**, focusing on compilers, AddressSanitizer (ASan), build systems, and linters. - The **codebase side**, responsible for core libraries such as `folly` and an internal directory literally called `common`.

Because this team sits at the intersection of both the sanitizers and the foundational libraries, they serve as the primary escalation point for severe engineering issues. Brandy explains that he regularly participates in the site event review meetings when something takes down Facebook. “So when something crashes the site, especially if it involves a C++ bug, I see it,” he notes, explaining that developers also frequently reach out wondering if an AddressSanitizer alert is a false positive, only to be told: “No, no it's not, you have a bug in your code and it's subtle and it's here.”

---

[02:12](https://www.youtube.com/watch?v=lkgszkPnV8g&t=132s) Talk Criteria, Tooling Bias, and Two Audiences

Brandy outlines four strict criteria for the bugs featured in the presentation: 1. **Scary:** No mundane syntax errors or trivial compiler warnings. 2. **Common and recurring:** Issues that continuously crop up despite extensive documentation, education, and tooling. 3. **Concrete:** Focused on specific code snippets that fit on a slide, avoiding high-level architectural complaints like vague critiques of library ownership models. 4. **Not just edge cases:** Not a compilation of the single craziest bugs ever encountered, which would require an entirely separate talk.

Mitigation strategies will be highlighted for every bug, with a heavy emphasis on runtime sanitizers. Brandy gives fair warning about his personal bias toward the GCC/Clang and Linux tooling ecosystem, noting that developers on other platforms should seek out equivalent tooling.

Addressing the room, he divides his listeners into two distinct camps: - **Audience 1 (Learners / Less experienced engineers):** Engineers who will see something brand-new, potentially sparing them from what Brandy calls “the worst week of your career.” - **Audience 2 (Experts and reviewers):** Seasoned veterans who have seen these exact patterns a hundred times in code reviews while cautioning their peers. Brandy jokes that experts claim they never write such bugs themselves: “Never, never, never. This never happens.” For this group, the presentation serves as an open inquiry into why these traps persist, and how the language, compiler tooling, and library design can evolve to eradicate them completely.

---

[04:35](https://www.youtube.com/watch?v=lkgszkPnV8g&t=275s) The #1 Cause of Bugs: Vector Subscripts and Out-of-Bounds Access

Brandy admits that from pure volume alone, it would be intellectually dishonest to omit the single largest cause of production incidents at Facebook: `std::vector::operator[]`, along with the equivalent C-style array indexing and raw pointer dereferencing. It represents the most frequent bug category across practically all software systems. However, he deliberately categorizes it as uninteresting for the remainder of the presentation because it is not unique to C++. Computing as a whole suffers from bounds-checking challenges, making it an inherent problem rather than a self-inflicted flaw unique to C++.

```mermaid flowchart LR accTitle: Bounds Issue Mitigations accDescr: Three approaches to mitigating vector bounds bugs in modern C++.

subgraph Mitigations["Mitigation Strategies"] direction TB A["Static Analysis<br/>(Expensive, interprocedural)"] B["Modern Abstractions<br/>(Range-based algorithms hoist checks)"] C["Dynamic Runtime Checks<br/>(AddressSanitizer + Fuzzing)"] end ```

Brandy points out that typical ways to trigger a site-wide crash include failing to check vector bounds before indexing, or iterating over two vectors while falsely assuming they share identical lengths. In discussing mitigations, he notes: - **Static Analysis:** Long considered the “queen mother of hard static analysis problems,” it remains computationally expensive and struggles with interprocedural reasoning at scale. - **Better Abstractions:** Shifting away from index-based loops toward range-based algorithms often enables the compiler to hoist bounds checks out of hot execution loops. - **Runtime Checking:** Languages like Ada enforce checks at every single access, but in C++, dynamic instrumentation remains paramount.

Brandy delivers an unambiguous recommendation for C++ engineering teams: “If you take nothing away from today, what you need to take away is `-fsanitize=address`.” AddressSanitizer paired with fuzzing remains the single most impactful tooling development in Facebook's modern development pipeline.

---

[07:03](https://www.youtube.com/watch?v=lkgszkPnV8g&t=423s) Bug #2: The Greatest Newbie Trap — `std::map::operator[]`

Setting up the bugs in rough chronological order of when an engineer typically encounters them during their career, Brandy nominates `std::map::operator[]` as the undisputed greatest newbie trap in the entire C++ language. He invites audience members who disagree to pitch their own candidates after the presentation.

To illustrate the flaw, he walks through a deceptively ordinary example:

```cpp std::map<std::string, int> my_map; my_map["hey"] = 12;

// Programmer typo: accidentally looking up "heyy" instead of "hey" std::cout << my_map["heyy"] << std::endl; ```

Most experienced C++ developers take it for granted that this prints `0`, but to any programmer coming from another language, that behavior is entirely counterintuitive: - It does not segfault or crash. - It does not read uninitialized garbage memory. - It does not throw a `std::out_of_range` or `KeyError` exception.

Instead, the subscript operator quietly default-constructs and inserts an entry for `"heyy"`, returning a reference to the newly created value, leaving the map with a size of `2`.

Brandy points out that experienced developers take advantage of this quirk without a second thought to implement idiomatically brief frequency counters:

```cpp std::map<char, int> counts; for (char c: str) { counts[c]++; } ```

While concise, this pattern is horrifying to newcomers. In languages like Java, developers are forced to check for a key's existence and explicitly initialize counters. When junior C++ engineers encounter the map idiom, they immediately ask awkward, fundamental questions about why integers are mysteriously set to zero without an explicit assignment. Brandy humorously notes that explaining why this works drags engineers into the painful weeds of C++ value initialization: “I want no part of like, value initialization... not with someone who's falling into this trap.”

[09:29](https://www.youtube.com/watch?v=lkgszkPnV8g&t=569s) The Const Correctness Trap and Site Incidents

Facebook tries to protect junior developers from accidental insertions into maps by strongly emphasizing const correctness: "Be const correct, it will save you from bugs, this is one of those bugs that saves you from." However, this safety net frequently fails because of how programmers respond to compiler errors. When a developer attempts to read from a `const std::map` using the subscript operator (`operator[]`), the compiler refuses to compile the code because `operator[]` mutates the container by inserting a default-constructed element if the requested key is missing.

When beginners run into the compiler error—typically formatted as `error: passing 'const std::map<...>' as 'this' argument discards qualifiers`—they interpret the message literally: passing a `const` map causes an error, so the immediate fix is simply to delete `const`. Once the qualifier is removed, the code compiles cleanly, leaving the developer under the impression that the issue is resolved while silently re-enabling dangerous in-place mutations.

This pitfall is not limited to junior engineers; it has caused severe production outages written by senior staff. Louis Brandy recounts: “Twice, twice as far as I know, major site incidents have occurred in exactly this pattern.” In one recurring architecture, an experienced team created a service handler widget whose constructor accepted a map of configuration settings. The code assigned the passed-in map to an internal member and then immediately printed diagnostic logging, such as `Server initialized with timeout:...`, accessing the value via `settings["timeout"]`. If an upstream refactor passed a settings map lacking a `"timeout"` entry, the logging statement itself inserted a default value of `0`. In typical socket libraries, a timeout value of zero denotes an infinite timeout. When an upstream dependency degraded or failed to respond, every thread in the dependent service became permanently blocked in an infinite wait.

- When asked how a beginner fixes code that fails on `const` map access, the audience responded: "Remove const." - The `operator[]` on an associative container default-constructs missing values (e.g., setting an integer timeout to `0`). - Because teams rarely test timeout and fallback failure paths thoroughly, zero-timeout bugs often escape directly into production. - Louis Brandy notes that Facebook engineering leadership has felt completely powerless against this anti-pattern, to the point where engineers have genuinely debated banning `std::map::operator[]` altogether.

---

[12:35](https://www.youtube.com/watch?v=lkgszkPnV8g&t=755s) Designing `findWithDefault`: Copies vs. Dangling References

When developers seek alternatives to `operator[]`, the standard library options carry trade-offs. While `.at()` exists and throws an exception when a key is absent, developers often dislike exceptions or simply fail to use it. Many other programming languages provide a "get with default" mechanism that returns a fallback value if a lookup fails. Louis Brandy compares how GHC Haskell implements `findWithDefault` in its standard prelude using pattern matching (`findWithDefault default k m`) against Facebook’s C++ utility in the Folly library. While Haskell uses straightforward type inference, the C++ equivalent requires substantial template boilerplate to inspect types, perform an iterator lookup (`find`), check against `end()`, and either return the stored value or a default.

```mermaid flowchart TD accTitle: findWithDefault Return-Type Dilemma accDescr: Flowchart illustrating the trade-off between returning by value versus returning by reference in a dictionary lookup with a fallback.

subgraph ByValue["Pass/Return by Value"] A1["Map Lookup: key not found"] --> B1["Return default by value"] B1 --> C1["Safe: Creates an independent copy"] C1 --> D1["Complaint: Extraneous string copy overhead"] end

subgraph ByReference["Return by Reference (const string&)"] A2["Map Lookup: key not found"] --> B2["Caller supplies temporary: find(m, k, string('default'))"] B2 --> C2["Return const reference to temporary"] C2 --> D2["Catastrophic: Reference binds to destroyed temporary (Dangling Reference)"] end ```

The primary grievance C++ developers raise against Folly’s original lookup helper is performance: when instantiated for types like `std::string`, returning by value forces an unnecessary, extraneous copy of the data. To eliminate that copy, developers repeatedly submit patches that alter the function signature to return `const std::string&`. Louis Brandy warns against this change emphatically: “This code is hopelessly broken. Do not copy and paste this code.”

Because callers overwhelmingly invoke lookup helpers using temporary defaults (such as string literals implicitly converting to a temporary `std::string`), returning a `const&` binds the caller's reference to an argument temporary that gets destroyed as soon as the full expression completes. The caller is left holding a dangling reference.

- The pull request changing the return type of `findWithDefault` to a reference was submitted roughly four times at Facebook; it was rejected three times and accidentally merged once, requiring immediate rollback. - This pattern represents a broader hazard where passing a temporary to a function that returns a reference silently introduces use-after-free bugs. - AddressSanitizer (ASan) reliably detects this issue, though historically it required enabling a non-default flag (stack-use-after-return / stack-use-after-scope checks). - When Facebook enabled that ASan detection flag across their test suites, they uncovered dozens of existing dangling reference bugs of this exact pattern across the codebase.

---

[17:08](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1028s) Intermission: Eradicating `volatile` in Multithreaded Code

Switching from persistent pitfalls to a tooling success story, Louis Brandy describes a bug pattern that Facebook successfully eliminated. The dispute centered around what he labels the worst keyword in all of C++: `volatile`. While `volatile` has valid uses in low-level systems programming—such as memory-mapped I/O or interactions with signal handlers—those patterns are virtually nonexistent in Facebook's application and server infrastructure. Instead, engineers historically reached for `volatile` under the mistaken belief that it provided thread synchronization primitives.

To eradicate this misconception, the team introduced an intentionally aggressive linter rule. Whenever an engineer introduced the keyword `volatile` outside of a comment, the linter flagged the revision and dumped an entire explanatory paragraph onto the code review interface. Although Louis Brandy acknowledges that printing long, non-actionable lectures violates standard linter design principles, the tactic was necessary to confront deep-seated legacy habits. Engineers frequently pushed back on reviews, insisting they were using it correctly to prevent compiler optimization across two threads communicating via a raw boolean flag because "we've been doing this for 20 years." Despite fierce pushback, the strict lint rule held, successfully driving `volatile` out of multithreaded application code across the entire repository.

- Audience members initially guessed `auto` and `goto` before arriving at `volatile` as the worst keyword. - Louis Brandy observed about developer confidence: "Lots of people think they know what it does. At least historically, and use it really really wrong, really really wrong." - The linter rule was unapologetically opinionated: "It's not actionable, which makes it a terrible lint rule, but I don't care, if you use volatile I'm going to lecture you." - Regarding the prolonged debates over thread synchronization and legacy practices, Brandy concluded: "We had so many of these arguments, but the thing that's interesting is that we won. We won the arguments."

[19:17](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1157s) How the "Great Volatile Wars" Ended

Louis Brandy reflects on how C++ usage has shifted over time at Facebook. Newer engineers who have arrived in recent years have no memory of the fierce debates that used to consume the engineering teams: "The Great Volatile Wars of 2010, they're gone, 2011." Back then, developers routinely reached for `volatile` as a synchronization primitive across threads. Today, that pattern has almost completely vanished.

Brandy observes that this shift serves as an instructive case study in developer behavior. Engineers were not using `volatile` because they loved the keyword; they used it because they desperately needed atomic flags, mutexes, or higher-level primitives to coordinate threads. Once the language and standard libraries began providing those first-class abstractions, engineers naturally adopted them. The older generation was retrained, while newer engineers entering the language simply found the right abstractions already waiting for them. As Brandy summarizes the dynamic: “People really want to do x, and they'll find a way to do x. And maybe if you just give them a solution, they'll use it, turns out.”

[20:08](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1208s) The Worst Question in C++: "Is `shared_ptr` Thread-Safe?"

Brandy introduces "bug number five," which stems from what he calls the single worst question in all of C++—one that genuinely makes his blood run cold. At Facebook, this issue invariably arrives via a familiar, agonizing story posted to an internal engineering group. A team reports that their service has been intermittently crashing in production for a month. Because the bug cannot be reproduced locally or in staging environments, the team has been forced to bisect the codebase directly in production: deploying a binary, letting it run under real traffic for six hours, and checking whether it survives without crashing.

After roughly three weeks of production bisection, the team finally narrows the failure down to a single commit diff. Yet, after staring at and pulling apart the diff, they still cannot see any obvious flaw. Desperate to validate their only working hypothesis, they approach the core library team and ask three simple words: "Is `shared_ptr` thread-safe?"

When Brandy asks the audience for their reaction, he receives mixed shouts of "No!", "Yes!", and "It depends!" Another classic answer is, “It's exactly as thread-safe as a normal ptr”—a response Brandy characterizes as technically true, but practically useless. A quick Google search for the phrase "is shared_ptr" proves how pervasive the confusion is: - The #1 auto-complete search is: **"is shared_ptr thread-safe?"** - The #3 search is: **"is shared_ptr slow"** (Brandy’s retort: “No, no shared_ptr is not slow. If it's in your profile, fix your code.”) - The #4 search is: **"is shared_ptr null"** (Brandy's answer: “Yes. Yes, shared_ptr is null. Put a check in.”)

When asked whether `shared_ptr` is thread-safe, Brandy has learned to give a blunt answer: “I believe the answer to, is shared_ptr thread-safe, to be no.” While technically a white lie from a language-specification perspective, it is the only safe answer in practice: “You asked, is shared_ptr thread-safe, but what you meant to ask was, is my code correct? And the answer, if you have to ask, is no.”

[22:38](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1358s) Mental Model: The Three Parts of a `shared_ptr`

To explain why developers get burned, Brandy lays out a simplified mental model of how `std::shared_ptr` operates. Rather than getting bogged down in low-level library implementation details, he conceptualizes a `shared_ptr` as comprising three core parts:

1. **The `shared_ptr` struct itself:** Under the hood, this is essentially a pair of pointers living on the stack or inside a container. 2. **The Control Block (the shared reference count):** One pointer in the struct points here. Although committee purists will protest that it contains custom deleters, allocators, and weak counts, Brandy treats it simply as the atomic reference counter. 3. **The Shared Object (`T`):** The second pointer points directly to the managed payload instance.

```mermaid flowchart LR accTitle: Mental Model of std::shared_ptr accDescr: Diagram showing a shared_ptr struct containing two pointers: one pointing to the shared object T, and the other pointing to the control block containing the atomic reference count.

subgraph Instance["shared_ptr Struct (Pair of Pointers)"] P1["Pointer to T"] P2["Pointer to Control Block"] end

subgraph Heap["Shared Heap Allocations"] Obj["Shared T\n(Unsynchronized)"] Ctrl["Control Block / Ref Count\n(Thread-Safe / Atomic)"] end

P1 --> Obj P2 --> Ctrl ```

When a `shared_ptr` is copied, the struct's pair of pointers is duplicated to point to the existing control block and object, and the atomic reference count inside the control block is incremented. From a thread-safety perspective, these three components have completely different guarantees: - **The Shared Object (`T`):** Completely unsynchronized. The `shared_ptr` wrapper provides zero synchronization for the underlying data; protecting concurrent reads/writes to `T` is entirely the user's responsibility. - **The Control Block / Reference Count:** Completely synchronized. Multiple threads can safely make copies and destroy their local `shared_ptr` instances concurrently; the control block atomically increments and decrements the counter. - **The `shared_ptr` Struct Itself:** Completely unsynchronized. Just like a plain C-style raw pointer or a `std::pair<void*, void*>`, two threads cannot read and mutate the exact same `shared_ptr` instance simultaneously without external synchronization.

Programmers naturally intuit that concurrent mutations to `T` require locks, and they correctly assume the reference count is thread-safe. Where their intuition fails—often resulting in catastrophic bugs—is assuming they can read from and write to the same `shared_ptr` variable across threads without a lock. While testing this slide with colleagues at Facebook to see if it was too elementary, an engineer suddenly had an epiphany: “One person literally went, that's the crash. That's the crash. We have a map, and the shared_ptrs, that's the crash.”

[25:17](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1517s) Degenerate RCU and the Race to the Control Block

At Facebook, the vast majority of `shared_ptr` thread-safety bugs originate in backend services that attempt an ad-hoc, degenerate implementation of Read-Copy-Update (RCU). A typical service architecture follows a simple pipeline: - A background updater thread wakes up periodically (e.g., every 20 minutes), queries a database, constructs a fresh, highly optimized in-memory snapshot of that data, and assigns it to a global or shared `shared_ptr` representing the latest state of the world. - Incoming worker threads process client requests by grabbing a copy of that `shared_ptr` at the start of each request. They increment the reference count, safely pin that specific snapshot for the duration of the query, and let it fall out of scope when finished.

```mermaid flowchart TD accTitle: Degenerate RCU Pattern with shared_ptr accDescr: Flowchart illustrating an updater thread replacing a shared pointer state while worker threads copy it to process requests.

DB[("Database")] -->|"Wake every 20m & query"| Updater["Updater Thread\n(Builds new state)"] Updater -->|"Unsynchronized write (BUG!)"| GlobalPtr["Global shared_ptr State"]

GlobalPtr -->|"Unsynchronized read/copy (BUG!)"| Worker1["Worker Thread 1\n(Pins State A)"] GlobalPtr -->|"Unsynchronized read/copy (BUG!)"| Worker2["Worker Thread 2\n(Pins State B)"] ```

While this design has excellent performance properties—allowing instant snapshot handoffs without blocking active reader threads—developers repeatedly fail at the handoff. They attempt to write to the shared `shared_ptr` from the updater thread while worker threads concurrently copy it, completely omitting a mutex: “The implementation here is actually really easy, all you gotta do is at the heart of that handoff, have a shared_ptr and a mutex. And have an atomic get and an atomic set and you're done.”

Without that mutex, the struct's internal pointers are updated non-atomically, leading to torn reads, corrupted internal pointers, double frees, or premature deletions. - **ThreadSanitizer (TSan):** Reliable at pinpointing this data race directly if teams run it under test workloads. - **AddressSanitizer (ASan):** Often catches the eventual symptom when the corrupted `shared_ptr` triggers an illegal memory access or heap corruption, but it rarely points directly to the originating race condition, leaving engineers baffled.

Brandy notes that the committee introduced free-function atomic overloads (`std::atomic_load`, `std::atomic_store`) for `shared_ptr` in C++11, but implementation support was historically spotty. Later revisions explored dedicated types like `std::atomic_shared_ptr` or `std::atomic<std::shared_ptr<T>>` (discussed at the Toronto committee meeting). After asking SG1 concurrency experts about its standardization history, Brandy found that the back-and-forth was an exhaustively complex saga. Regardless, he stresses that service authors do not need exotic, high-performance lock-free atomic pointers to fix the problem: a simple `std::mutex` wrapping the handoff or an upcoming cell-type abstraction completely eliminates the bug.

Finally, Brandy flags a related "bonus bug" involving `shared_ptr` lifetime confusion that surfaces repeatedly in the Facebook codebase: a function returns a `shared_ptr` by value as a temporary, and the caller immediately dereferences it to grab a raw reference or pointer to the underlying object (`const auto& ref = *getSharedThing();`), creating an instant dangling reference once the temporary `shared_ptr` is destroyed at the end of the full-expression.

[28:43](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1723s) Dereferencing Temporary `shared_ptr` and AddressSanitizer

Wrapping up the previous discussion on reference counting, Louis Brandy explains what goes wrong when calling a member function on a temporary `std::shared_ptr`:

```cpp ref.boom(); ```

Because the `std::shared_ptr` was dereferenced immediately, its lifetime ends as soon as the temporary expression is evaluated. The object itself is destroyed on the spot, stripping away all reference-count protection from that point forward. As Brandy summarizes, “And you basically, you don't have the protection of a reference count anymore, right? So all your code is now running without a reference count protecting you.” This pitfall has surfaced repeatedly across Facebook's codebase. Fortunately, AddressSanitizer (ASan) reliably and easily catches this use-after-free pattern at runtime.

[29:10](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1750s) Bug #6: The Most Ambiguous Declaration Quiz

Brandy transitions to his favorite bug of the presentation: Bug Number Six. He sets up an interactive code-review exercise consisting of a series of minimal code snippets, asking the room two core questions: 1. Does the code compile? 2. If it does not compile, what error message would you expect (or wish for)?

He displays the first test case:

```cpp #include <string>

void f() { std::string(foo); } ```

Only about 10% of the room raises their hands to say it compiles. However, sitting directly in the audience is Richard Smith, the editor of the C++ standard. Brandy points him out to the room: “The editor of the C++ standard says this code compiles. Who's changed their mind?” Richard Smith is correct: the code compiles cleanly, fooling roughly 90% of the audience. Brandy jokes that viewers watching at home should pause and contemplate existentially how this is legal and why they are wasting their lives on C++.

The explanation comes down to C++'s disambiguation rules: if a statement can be parsed as a declaration, it *must* be treated as a declaration regardless of what else it resembles. In this statement, the parentheses around `foo` are purely redundant syntactic groupings:

```cpp std::string(foo); // Identical to: std::string foo; ```

This constructs a local `std::string` named `foo`. Checking the snippet in Compiler Explorer (Godbolt) with optimizations disabled proves it: the compiler emits a call to default-construct an empty `std::string foo`, followed by a call to its destructor upon exiting scope.

[31:42](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1902s) The Phantom Lock Guard Bug

While `std::string(foo);` looks like an obscure piece of language trivia, Brandy warns that it underpins one of the most terrifying, recurring concurrency bugs in C++. He illustrates how the bug typically manifests inside a multithreaded class:

```cpp class SynchronizedObject { std::mutex my_mutex;

void update() { std::unique_lock<std::mutex>(my_mutex); // Mutate shared state... } }; ```

Only about half the audience spots the error immediately. The developer intended to create an unnamed temporary RAII lock guard that locks `my_mutex` on construction and releases it on destruction. Instead, the code declares a brand-new local variable of type `std::unique_lock<std::mutex>` whose name is `my_mutex`. This local variable shadows the class member `my_mutex`, default-constructs itself without taking any lock whatsoever, and does not synchronize anything.

```mermaid flowchart TD accTitle: Unnamed unique_lock Declaration Pitfall accDescr: Diagram illustrating how std::unique_lock(my_mutex) parses as a declaration shadowing the member mutex rather than creating an RAII lock.

subgraph Intended["Developer Intent"] A["Temporary lock guard"] -->|"Locks"| B["Member: my_mutex"] A -->|"Unlocks at end of line"| B end

subgraph Actual["C++ Grammar Parse"] C["std::unique_lock<std::mutex>(my_mutex);"] C --> D["Declares variable named 'my_mutex'"] D --> E["Shadows member variable"] D --> F["Calls default constructor: NO LOCK ACQUIRED"] end ```

The fix requires explicitly providing a variable name for the guard object:

```cpp std::unique_lock<std::mutex> g(my_mutex); ```

Brandy urges everyone to run a search across their own repositories for patterns resembling `std::unique_lock(...)`. While GitHub search limitations prevented him from scanning all public open source, he audited Facebook's repository: - He found two distinct occurrences of `std::unique_lock(mtx);` silently failing to lock their targets. - These bugs persisted despite Facebook already having an active linter rule designed to detect this exact failure mode. - Brandy laments how many hundreds of these would exist in the wild without automated linting to block them.

[34:34](https://www.youtube.com/watch?v=lkgszkPnV8g&t=2074s) The "Worst C++ Quiz of All"

Brandy walks through four rapid-fire variations of parenthesized and braced expressions, demonstrating how grammar rules differentiate between declarations, temporary objects, and redefinitions:

```cpp // 1. Declaration vs. Temporary Object (Compiles) std::string(foo); // Declares variable 'foo' of type std::string std::string("hello"); // Compiles: Constructs temporary std::string from const char* ```

```cpp // 2. Duplicate Declarations (Fails to compile) std::string(foo); std::string(foo); // Compile Error: Redefinition of 'foo' in the same scope ```

An audience member asks whether the second line could be interpreted as creating a temporary copy of `foo`. Brandy clarifies that because the grammar dictates that any statement ambiguous with a declaration must be resolved as a declaration, the compiler treats both lines as `std::string foo;`, causing a redefinition error.

```cpp // 3. Declaration Followed by Temporary Copy Construction (Compiles) std::string(foo); (std::string(foo)); // Compiles: Outer parentheses force an expression, // invoking the copy constructor to create a temporary string ```

```cpp // 4. Nested Block Scope Shadowing (Compiles) std::string(foo); { std::string(foo); // Compiles: Declares 'foo' in an inner scope, shadowing the outer 'foo' } ```

[37:14](https://www.youtube.com/watch?v=lkgszkPnV8g&t=2234s) Anatomy of the Vulnerability: RAII and Default Constructors

Brandy identifies the two specific conditions that conspire to make `std::unique_lock` exceptionally prone to this disaster:

1. **RAII Semantics**: Non-RAII objects usually demand a name because developers reference them in later expressions; forgetting the name causes an immediate compile error. With an RAII guard, however, acquisition happens purely through creation. Developers subconsciously assume declaring the type and argument is sufficient. 2. **The Default Constructor**: The shadowing declaration `std::unique_lock(my_mutex)` only compiles because `std::unique_lock` provides a default constructor (which creates an unassociated, unlocked object).

By contrast, `std::lock_guard` does not have a default constructor; writing `std::lock_guard<std::mutex>(my_mutex);` fails to compile because the compiler attempts to default-construct a variable named `my_mutex`.

- Standard `std::lock_guard` is immune to this bug because it lacks a default constructor. - Facebook's codebase contained dozens of custom in-house `lock_guard` reimplementations written over the years. - Because engineers rarely appreciated this parsing hazard, many custom implementations included default constructors, leading to "tens, maybe 100 cases" of phantom locks across Facebook's internal classes. - Brandy gives an urgent directive to library authors: “So if you have a lock_guard, run, don't walk, to see if it has a default constructor.” - Turning to C++17's `std::scoped_lock`, Brandy notes that its vulnerability remains unclear: when testing in Compiler Explorer the day prior, Clang rejected the equivalent syntax while GCC accepted it.

[039:12] Default Constructors, RAII Pitfalls, and Shadowing Warnings

Louis Brandy wraps up the discussion on RAII edge cases, observing that while C++17 might change certain behaviors, the fundamental lesson stands: “So anyway, the TL;DR on all this is if you have an RAII type and it has a default constructor, you're in a very dangerous situation, potentially, where people will try to wrap something and forget to name the object instead of what they're wrapping and they'll end up shadow... Declaring a shadowing thing.” When developers write RAII wrappers without explicitly naming the instance—accidentally reusing an existing identifier—they create a shadowing variable declaration rather than a properly scoped guard.

Turning to mitigations, Brandy evaluates the compiler flags in GCC and Clang. The classic flag `-Wshadow` catches this exact bug because it warns whenever an inner-scope variable shadows one from an outer scope. However, in practice, `-Wshadow` is notoriously pedantic and almost universally disabled due to deafening noise and low-value warnings that often make code worse. To address this, compilers introduced higher-signal hybrid flags: - `-Wshadow-compatible`: Warns only if the shadowing types are compatible with one another. - `-Wshadow-compatible-local`: Restricts warnings strictly to compatible types declared locally within the same function scope.

Brandy strongly recommends enabling `-Wshadow-compatible-local`, which reliably identifies smelly patterns—such as an outer loop index `i` being shadowed by inner loop indices named `i`. Nevertheless, there is bad news: because RAII wrappers (like locks) and the mutexes they guard are incompatible types, neither `-Wshadow-compatible` nor `-Wshadow-compatible-local` will flag the missing-lock shadowing bug. Only full `-Wshadow` catches it.

```mermaid flowchart TD accTitle: Compiler Shadowing Flags Comparison accDescr: Diagram comparing different GCC and Clang shadowing warning flags and their effectiveness against RAII shadowing bugs.

subgraph Flags["Compiler Warning Flags"] direction TB WFull["-Wshadow"] WCompat["-Wshadow-compatible"] WLocal["-Wshadow-compatible-local"] end

subgraph Results["Diagnostic Capability"] direction TB CatchBug["Catches RAII Mutex Shadowing Bug<br/>(High false-positive noise)"] MissBug["Misses Incompatible Types<br/>(Clean signal, catches loop index collisions)"] end

WFull --> CatchBug WCompat --> MissBug WLocal --> MissBug ```

[041:01] Custom Linters and the Nodiscard Alternative

Because standard compiler warnings leave a gap, Facebook relies on Clang-based linters with custom AST checks to flag unnamed lock guards that immediately wink out of existence. Brandy notes that unless an engineering organization has dedicated tooling targeting this specific idiom, codebases using `std::unique_lock` or custom default-constructible guards are likely riddled with silent concurrency bugs.

Beyond declaration errors, Brandy highlights a related flavor of bug where a temporary object is constructed to guard a scope, only to be destroyed at the end of the single expression line because the developer forgot to assign it to a variable. While the declaration-shadowing bug remains far more frequent, an attribute such as `[[nodiscard]]` applied to constructors could help compilers diagnose these single-line throwaways in the future. In the meantime, tooling like `clang-tidy` remains the primary line of defense.

- Unnamed lock guards instantly destruct at the end of the statement or declaration, leaving critical sections completely unprotected. - Standard libraries with default constructors on synchronization primitives dramatically increase vulnerability to accidental re-declarations. - Custom AST checks in tools like `clang-tidy` easily identify RAII objects whose lifetime fails to cover their intended block.

[042:03] The High Cost of Trivial Bugs and the Education Hierarchy

Reflecting on the nature of these defects, Brandy candidly addresses the reality of the language: “C++ is hard. Like a lot of this stuff here, we've had some of the best C++ people spend a week finding a bug, and at the end track it down to, like, their lock_guard is missing a g.” Discovering that an entire week of intense debugging culminates in adding a single missing character is excruciating.

To prevent these scenarios, Brandy champions automated tools over lectures and documentation. While internal classes and talks are common, their effectiveness drops drastically the further away they are from the moment code is written:

```mermaid flowchart LR accTitle: Feedback Latency and Value Hierarchy accDescr: Pipeline showing the diminishing returns of developer feedback as time from commit increases.

subgraph FastFeedback["High Value: Immediate"] IDE["IDE / Compiler Diagnostics"] CodeReview["Async Code Review Bots"] end

subgraph SlowFeedback["Low Value: Delayed"] Nightly["Interprocedural Batch Scanners"] Lectures["Tech Talks (Years Later)"] end

IDE -->|"Fastest"| CodeReview CodeReview -->|"Days"| Nightly Nightly -->|"Years"| Lectures ```

As Brandy illustrates, an instant warning from an IDE or compiler is golden; an automated review bot commenting on a diff is very good; an email from an expensive interprocedural static analyzer two days after commit is tolerable; but a tech talk explaining a bug two years after an engineer spent a week debugging it is “borderline useless.”

[044:01] Facebook Tooling and AddressSanitizer Evangelism

Brandy details the specific internal tooling that Facebook deploys. In addition to standard tools like `clang-tidy`, Facebook built an extendable Clang-based linter playfully named `how-to-even` (inspired by the colloquial phrase "I can't even"). This linter enforces strict codebase invariants, including an explicit prohibition against `std::lock_guard` variables shadowing the mutexes they are meant to protect.

Brandy turns to AddressSanitizer (ASan), admitting that while he did not intend for it to be the silver bullet for every issue in his talk, its effectiveness is unrivaled: “One cannot speak highly enough of Address Sanitizer, and the amount of bugs it finds in a codebase. If you live in a codebase that compiles with Clang or GCC, and you've never turned Address Sanitizer on, leave. Go do it right now.” He jokes that developers should run their test suites with ASan enabled, keep quiet about the dozen critical bugs they instantly discover, ask for a raise first, and only then teach their teams how to use it.

[045:08] Q&A: Lifetimes, Deleted Overloads, and Optional Map Lookups

During the open question period, an audience member asks which bugs failed to make the presentation's final cut. Brandy replies that many suggested unnecessary copies or small passing-convention preferences, but he rejected those because obsessive fear of copies causes even worse bugs. Instead, the most serious omitted issue is asynchronous lifetimes: with `folly::Future` and fibers, tracking asynchronous execution lifetimes without a garbage collector is notoriously difficult, though upcoming C++ coroutines may eventually alleviate the burden.

Another attendee asks why Facebook did not simply put a warning comment on the dangerous `get_default` map helper discussed earlier. Brandy reveals that they solved the issue by creating an rvalue overload explicitly marked `= delete` and adding explanatory comments:

```cpp // Prevent binding temporaries that produce dangling references template <typename Map, typename Key, typename Val> Val const& get_default(Map const& map, Key const& key, Val&& dflt) = delete; ```

The underlying complication was that the codebase contained duplicate implementations: while `folly` was quickly patched, an ancient, redundant version in an internal `STL utils` header silently introduced the bug before anyone noticed.

The session concludes with an exchange regarding associative lookups and copy elision: - A listener asks whether changing `std::map::operator[]` on const maps to return an optional reference (similar to Haskell's `Maybe`) would solve reference lifetime issues. Brandy agrees in principle, but another attendee points out that `std::optional` does not support references. - Asked whether mandatory copy elision in C++17 will ease anxiety about copies, Brandy expresses skepticism: “I think copy elision is good. On average, it'll be good. I think the broader problem of, it's really easy to smuggle a reference out of a function, is its own problem. I don't know how or if we can fix it.”

[49:04](https://www.youtube.com/watch?v=lkgszkPnV8g&t=2944s) Copy Elision Realities and In-House Clang Linters

Addressing an earlier question about copy elision in complicated code, Louis Brandy points out the practical limits of compiler optimizations when dealing with patterns like a fallback lookup function (`get_default`). Because there are multiple potential return sources—either the existing entry residing inside the map or a supplied default value—the compiler cannot simply elide the copy. If the desired value is inside the map, the function has no choice but to copy it out. While better language support and guaranteed copy elision might reduce developers' generalized paranoia about unnecessary copies on average, specific multi-source access patterns inherently mandate copying.

```mermaid flowchart LR accTitle: Lookup Flow in get_default accDescr: Diagram showing why get_default cannot elide copies due to multiple distinct return sources. subgraph Lookup["get_default(key, default_val)"] A["Check Map for Key"] -->|"Key Found"| B["Map Element Reference"] A -->|"Key Missing"| C["Fallback Default Value"] end B -->|"Must Copy Out"| D["Return Value Copy"] C -->|"Must Copy Out"| D ```

An audience member named Dmitri asks whether Facebook plans to open-source the custom Clang-based linters demonstrated during the presentation. Brandy explains that Facebook's internal tool was developed just prior to Clang-Tidy becoming mature and usable for their environment:

> “So we sort of built our own version of this thing, and there's been great long term discussion of, like, if we open source it, are we just bifurcating the space.”

Brandy expresses strong support for open-sourcing tooling in general, noting that Facebook frequently does so. However, because their internal utility is essentially just a simpler, earlier iteration of Clang-Tidy, the team is actively weighing whether it makes more sense to rewrite and contribute all of their custom checks directly into upstream Clang-Tidy rather than releasing a competing, standalone framework.

- In multi-source branches like `get_default`, copy elision cannot trigger because the compiler must select between distinct storage locations at runtime. - Facebook created its in-house linting tool right before Clang-Tidy was widely available, leaving them with a dilemma between maintaining a bespoke tool or upstreaming checks to Clang-Tidy.

---

[50:17](https://www.youtube.com/watch?v=lkgszkPnV8g&t=3017s) The Cost of Copies at Scale vs. Micro-Optimization Paranoia

An audience member offers an observation regarding developer perspective and scale, remarking to the room:

> “When Facebook copies 100 bytes too many, it costs them one million dollars. Me and you, it costs us zero. You have so much bigger problems than just copying a string one too many times.”

Brandy readily concedes the point, acknowledging that hyper-focusing on small copies is frequently unnecessary in everyday software engineering. However, he highlights the dilemma from the perspective of an infrastructure or library maintainer: library authors constantly face users who obsess over potential overhead. Brandy humorously notes that even among the users who claim to care intensely about micro-benchmarks, they often care far too much about allocations that fail to show up in any meaningful production profile:

> “It's like, no, I don't see it in your profile, go away.”

- At massive hyperscale deployments like Facebook, even minor memory overheads or tiny redundant copies can translate into substantial aggregate infrastructure costs. - For most developers outside that scale, obsessing over single string copies represents premature optimization that yields zero measurable business value. - Infrastructure and library developers are often pushed into micro-optimizations simply to satisfy users who scrutinize interface overhead regardless of actual workload profiles.

---

[50:58](https://www.youtube.com/watch?v=lkgszkPnV8g&t=3058s) Scoped Lock Constructors, Standards Committees, and Wrap-Up

Tony van Eerd asks Brandy whether he has brought up the pitfalls surrounding lock constructors—specifically `std::scoped_lock`—with the ISO C++ standards committee. Brandy recounts that he initially discussed the problem with members of SG1 (the concurrency study group), who suggested filing a Defect Report (DR). Tony interjects to recommend taking it directly to the Library Evolution and Library Working Groups instead.

Brandy admits that after examining the issue more closely, determining the exact defect status is surprisingly intricate. While he initially suspected the standard constructor suffered from the exact same dangerous behavior as single-mutex guards, the problem is complicated by modern features:

> “Because it's a variadic template, so it actually, and then there's argument deduction now, so it's complicated.”

Because class template argument deduction (CTAD) and variadic templates interact in subtle ways, Brandy remains uncertain whether `std::scoped_lock` actually exhibits the bug in practice. An audience member points out that committee experts present in the room could answer the question on the spot, drawing attention to Richard Smith sitting in the audience. Brandy jokes, “I know, poor Richard, he didn't know what talk he was coming to,” before closing the session to audience applause.

- Discussions with SG1 pointed toward filing a Defect Report (DR), though guidance suggested consulting the Library Evolution Working Group (LEWG) and Library Working Group (LWG). - The interaction between variadic templates and C++17 Class Template Argument Deduction (CTAD) makes verifying the exact initialization behavior of `std::scoped_lock` far from trivial.

## Notable quotes > [04:35](https://www.youtube.com/watch?v=lkgszkPnV8g&t=275s) If you take nothing away from today, what you need to take away is -fsanitize=address. — Louis Brandy

> [50:17](https://www.youtube.com/watch?v=lkgszkPnV8g&t=3017s) When Facebook copies 100 bytes too many, it costs them one million dollars. Me and you, it costs us zero. You have so much bigger problems than just copying a string one too many times. — Audience member

> [37:14](https://www.youtube.com/watch?v=lkgszkPnV8g&t=2234s) So if you have a lock_guard, run, don't walk, to see if it has a default constructor. — Louis Brandy

## Mentioned Louis Brandy, Richard Smith, Tony van Eerd, Dmitri, Facebook, PHP, Hack, HHVM, Python, Java, AddressSanitizer (ASan), ThreadSanitizer (TSan), folly, GCC, Clang, Linux, Ada, GHC Haskell, Clang-Tidy, how-to-even, ISO C++ standards committee, SG1, Library Evolution Working Group (LEWG), Library Working Group (LWG)

## Video description http://CppCon.org — Presentation Slides, PDFs, Source Code and other presenter materials are available at: https://github.com/CppCon/CppCon2017 — I've spent the last few years watching Facebook's C++ codebase grow by several orders of magnitude. Despite constantly improving abstractions, constantly improving tooling, frequent internal courses, and ongoing internal discussion, there are bug-patterns we simply cannot stop from being reintroduced into our code. My hope is to show some of the most common (and infamous) bugs in our history, and the surprising complexity that arises in some apparently simple situations.

This talk serves the dual purpose of educating the intermediate (and perhaps the occasional advanced) C++ programmer about some really nasty common pitfalls, as well as serves as a plea to experts to help further improve the language, libraries, and best practices to help educate and eradicate some of these problematic patterns. — Louis Brandy: Engineering Director, Facebook

My team is responsible for the overall health of the Facebook C++ codebase, both the tools and the libraries. We work on: compilers, static/dynamic analysis, linters, large scale changes, and the core libraries. — Videos Filmed & Edited by Bash Films: http://www.BashFilms.com

Work at Hudson River Trading (HRT): https://tinyurl.com/safxfctf ---

Videos Filmed & Edited by Bash Films: http://www.BashFilms.com

[Source](https://www.youtube.com/watch?v=lkgszkPnV8g)

中文简报

隐蔽C++陷阱引发网站宕机,Facebook强制采用Sanitizer 由于C++语法陷阱和对拷贝的过度担忧会在无形中引入悬垂引用和幻象锁,开发者培训与代码审查均未能阻止故障反复发生,这使得自动化AST检查工具与AddressSanitizer成为唯一有效的防御手段。 Louis Brandy分析了尽管经过广泛的代码审查和培训,但仍困扰大型C++代码库的几类臭名昭著且反复出现的漏洞。他重点指出了若干细微陷阱,例如std::map::operator[]引起的非预期插入、将临时对象传入兜底查找辅助函数所产生的悬垂引用,以及围绕std::shared_ptr普遍存在的线程安全误解。Brandy还深入剖析了潜在的语法陷阱,例如因可默认构造的RAII守卫遮蔽现有互斥锁而导致的幻象锁声明。最终他指出,讲座和人工审查无法阻止这些陷阱,并主张采用AddressSanitizer等运行时插桩手段、自定义AST检查工具以及安全的API设计来进行防御。 AddressSanitizer(-fsanitize=address)仍然是消除释放后使用(use-after-free)和悬垂引用漏洞最有效的单一工具,其效果超过了文档和培训工作。 (4:35) 在键缺失时通过std::map::operator[]意外插入元素,曾通过插入永久阻塞线程的零值超时设置,导致Facebook多次发生全站级别的大规模宕机。 (9:29) 使用std::shared_ptr的自制RCU实现经常引发竞态条件,因为shared_ptr结构体本身在未同步的并发读写访问下并不是线程安全的。 (25:17) 语法消歧规则会将std::unique_lock<std::mutex>(mtx)等未命名锁声明解析为遮蔽互斥锁的局部变量声明,从而导致临界区未被上锁。 (31:42)

Detailed notes — zh

## Background 本视频由 CppCon 官方频道于 2017 年 11 月 2 日发布在 YouTube 上。根据页面介绍,演讲者为 Facebook 工程总监 Louis Brandy,他通过本场演讲向中高级开发者展示 Facebook 庞大代码库中屡禁不止的几种最常见、最臭名昭著的 C++ Bug 模式,并呼吁语言与工具链专家共同探索根除这些缺陷的最佳实践。 ## TL;DR Louis Brandy 结合 Facebook 线上生产事故复盘与核心工具团队的治理经验,深入剖析了若干高频且破坏力极强的典型 C++ 缺陷模式。演讲剖析了容器下标越界、`std::map::operator[]` 隐式插入默认值导致的服务死锁、`findWithDefault` 滥用常引用返回引发的临时变量悬垂生命周期崩溃,以及团队根除 `volatile` 关键字的演进历程。此外,他还详细拆解了围绕 `shared_ptr` 线程安全认知误区而导致的并发竞争,并揭示了语法解析歧义如何让未命名的 `std::unique_lock` 静默退化为遮蔽成员的假锁。最后,他强调了短反馈回路工具链与 AddressSanitizer、Clang 自定义 Linter 在大型工程防护中的不可替代性。 ## Atmosphere & style 整场演讲氛围风趣幽默却又直切痛点,充满了大厂核心基建老兵面对线上灾难时的自嘲与务实。演讲者与台下观众互动热烈,通过现场代码竞猜和拉出 C++ 标准委员会核心成员调侃,将枯燥严谨的语言语法规则变成了引人入胜的侦探解密过程。观看体验既像是一场直面语言设计暗坑的排雷实战课,又透着工程团队在巨大代码规模下与复杂性长年搏斗的无奈与坚守。 ## Bulletin - [00:00](https://www.youtube.com/watch?v=lkgszkPnV8g&t=0s) Facebook 核心 C++ 团队分享了生产环境中屡禁不止的高危 Bug 模式,指出容器越界和隐式类型行为仍是引发严重线上事故的核心根源。 - [11:06](https://www.youtube.com/watch?v=lkgszkPnV8g&t=666s) std::map 的 operator[] 因自动进行默认构造与隐式插入,在 Facebook 曾引发多次将超时时间设置为 0 并导致工作线程无限挂起的全站级故障。 - [31:42](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1902s) 具有默认构造函数的 RAII 类型(如 std::unique_lock)在不小心漏写变量名时,会被编译器静默解析为变量声明并遮蔽成员互斥量,造成临界区完全失效且无锁保护。 - [42:03](https://www.youtube.com/watch?v=lkgszkPnV8g&t=2523s) 团队通过严格推行静态检查器和 AddressSanitizer(ASan)建立了最短的错误反馈循环,彻底消灭了诸如滥用 volatile 等历史并发缺陷。 ## Key takeaways - [00:00](https://www.youtube.com/watch?v=lkgszkPnV8g&t=0s) Facebook 底层基础设施深度依赖 C++,核心团队通过主导线上故障复盘与 AddressSanitizer(ASan)分析,提炼出了工业界最常复现的高危代码模式。 - [04:35](https://www.youtube.com/watch?v=lkgszkPnV8g&t=275s) std::vector 的 operator[] 下标越界是代码库中数量最多的 Bug 根源,结合模糊测试(Fuzzing)与 AddressSanitizer 是当前工程实践中最立竿见影的防护手段。 - [07:03](https://www.youtube.com/watch?v=lkgszkPnV8g&t=423s) std::map 的 operator[] 在键不存在时会静默执行值初始化并插入新条目,导致无意间修改容器大小,是最具欺骗性的初学者陷阱。 - [09:29](https://www.youtube.com/watch?v=lkgszkPnV8g&t=569s) 初学者常因误读编译器关于 const 限定符被丢弃的报错,通过错误删除 const 修饰符来让代码强行编译,从而掩盖了 operator[] 的非受控修改行为。 - [11:06](https://www.youtube.com/watch?v=lkgszkPnV8g&t=666s) 日志打印中误用 operator[] 会将缺失的配置静默初始化为 0,网络套接字库将其解释为无限超时,曾引发导致线程全数卡死的重大级联故障。 - [12:56](https://www.youtube.com/watch?v=lkgszkPnV8g&t=776s) 为避免拷贝而将 findWithDefault 的返回值改为 const 引用,极易将传入的临时对象以悬垂引用形式逃逸,最终引发线上崩溃。 - [17:08](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1028s) Facebook 曾通过激进的 Linter 规则和严格推行 std::atomic,彻底终结了开发者试图用 volatile 实现多线程同步的错误认知。 - [20:08](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1208s) “shared_ptr 是否线程安全”问题的技术回答虽然复杂,但在业务工程中一旦提出该疑问,往往意味着开发者正试图在未同步的情况下并发读写同一个智能指针实例。 - [22:38](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1358s) shared_ptr 仅保证控制块的引用计数修改是原子的,共享对象自身与智能指针内部结构本身的并发读写均需要调用方显式进行互斥同步。 - [25:17](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1517s) 在退化版 RCU 架构中,若跨线程并发更新全局 shared_ptr 时未加锁,AddressSanitizer 仅能捕捉到内存破坏表现,需要 ThreadSanitizer 才能精确定位数据竞争。 - [29:10](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1750s) C++ 语法歧义规则规定“只要长得像声明就按声明解析”,导致 std::string(foo); 等价于 std::string foo;,并不构造临时变量。 - [31:42](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1902s) 写成 std::unique_lock(my_mutex); 会被编译器解析为声明名为 my_mutex 的局部变量,遮蔽了外层互斥量且不执行任何加锁,临界区处于裸奔状态。 - [37:14](https://www.youtube.com/watch?v=lkgszkPnV8g&t=2234s) 该加锁缺陷的根源在于 RAII 对象仅依赖声明和析构运作,结合其自带的默认构造函数,使得漏传对象名时不会触发任何缺少参数的编译报错。 - [39:12](https://www.youtube.com/watch?v=lkgszkPnV8g&t=2352s) 原生的 -Wshadow 告警因为噪声过大常被团队关闭,而 -Wshadow-compatible-local 虽能抓出局部循环变量复用,却无法检测未命名锁导致的成员变量遮蔽。 - [42:03](https://www.youtube.com/watch?v=lkgszkPnV8g&t=2523s) 防止代码缺陷的最优路径是缩短反馈回路:通过定制化的 Clang 扩展规则和在 CI 流程中全局启用 AddressSanitizer,能在代码合并前高效拦截隐藏隐患。 ## Detailed walkthrough ### [00:00](https://www.youtube.com/watch?v=lkgszkPnV8g&t=0s) Facebook 的 C++ 规模与核心团队背景 Louis Brandy 以轻松的开场感谢参会者顶着雨天在午餐后赶来。针对外部常有的疑问——“Facebook 真的有很多 C++ 吗?你们不是只用 PHP 吗?”——他指出,Facebook 实际上拥有极其庞大的 C++ 代码库,甚至从公司诞生之初就是如此,整个底层基础设施基本都由 C++ 构建。在内部,C++ 是仅次于 PHP/Hack 的第二大常用语言;而且运行 PHP/Hack 代码的虚拟机 HHVM 本身就是一个大型 C++ 进程,直接保存在主代码仓库中。此外,内部语言生态还包括第二梯队的 Python、第三梯队的 Java 以及其他各种处于长尾分布的编程语言。 Louis Brandy 所在的部门是 Facebook 的核心 C++ 团队,主要划分为两个方向: - **工具链方向(Tools)**:专注于编译器(Compilers)、各类 Sanitizer 工具、构建系统(Build Systems)等底层工程体系的建设与维护。 - **代码库与核心库方向(Codebase / Core Libraries)**:负责基础核心库的研发与沉淀,例如著名的开源库 folly,以及在内部直接被命名为 `common` 的底层基础库目录。 由于兼管动态检测工具(如 Sanitizer)与核心基础库,该团队往往成为公司内部各类技术疑难杂症的最终上报和仲裁中心。Louis Brandy 本人长期参与 Facebook 的线上事故复盘评审会(Site Event Review Meeting)。当线上服务发生故障或崩溃、特别是涉及 C++ 缺陷时,问题都会汇总到他面前。此外,工程师们怀疑遇到 AddressSanitizer(ASAN)“误报”时也会向他求助,而得到的回答通常都是冷酷的事实:“不,这不是误报,你的代码里确实藏着一个极其隐晦的 Bug。” 正是这些经历为他提供了大量真实的线上 Bug 样本。 ```mermaid flowchart TD accTitle: Facebook Core C++ Team Escalation Flow accDescr: 线上崩溃事件与 ASAN 告警汇聚到核心 C++ 团队进行事故复盘与代码根因诊断。 subgraph Sources["数据与问题来源"] A["线上服务崩溃<br/>(Site Outages)"] B["Sanitizer 告警 / 疑似误报<br/>(ASAN Reports)"] end subgraph CoreTeam["Facebook 核心 C++ 团队"] C["事故评审会<br/>(Site Event Review)"] D["工具链组<br/>(Compilers, ASAN, Build)"] E["基础库组<br/>(folly, common)"] end subgraph Outcome["技术沉淀与防护"] F["根因排查与代码修复"] G["高频危险模式归纳"] end A --> C B --> C C --> D C --> E D --> F E --> F F --> G ``` ### [02:12](https://www.youtube.com/watch?v=lkgszkPnV8g&t=132s) 遴选标准与两类目标受众 在挑选本次分享的 Bug 案例时,Louis Brandy 确立了严格的标准:首先,它们必须足够致命(Scary),普通的语法错误或拼写小错不值一提;其次,它们必须高频且反复出现(Common and Recurring),无论团队如何努力宣讲和防范,它们总能重新潜入代码库;最后,案例必须具体(Concrete),能够在幻灯片中用简短的 C++ 代码直观复现,而不是去空泛讨论“所有权模型设计混乱容易引发 Bug”这种抽象命题。 在防护与排查手段方面,Louis Brandy 承认本场演讲可能会显得像是在给 AddressSanitizer 站台:“我本来没打算把这场演讲办成 ASAN 的招聘宣讲会,但事实证明,ASAN 对我们今天将要讨论的几乎每一个 Bug 都具有极强的检测效果。”同时他也说明,由于 Facebook 内部基础设施深度依赖 Linux 平台,演讲中的工具讨论不可避免地带有 GCC/Clang 与 Linux 的生态偏向,使用其他工具链的开发者也应当配备相应等效的检测工具。 面对台下的听众,Louis Brandy 认为可以分为截然不同的两类群体,这让他感到既忐忑又兴奋: - **受众一(初学者或进阶工程师)**:可能看到从未见过的 Bug 模式。演讲希望通过提前揭示这些深坑,帮助他们避免未来在工作中踩雷,“我希望能展示至少一样大家从未见过的东西;如果你是学生或者初学者,这可能会帮你省下职业生涯中最糟糕的一整周排障时间”。 - **受众二(经验丰富的资深专家)**:台上的很多代码他们可能已经在 Code Review 里拦截过上百次,并且坚信自己绝对不可能犯这种低级错误。对于这部分专家,Louis Brandy 希望探讨更深层的机制:“我们之间的交流重心在于:为什么这些 Bug 会屡禁不止?我们能做些什么?我们能否通过改进语言标准、优化工具链体系,或是改进工程培训,从根本上彻底消灭这些模式?” ### [04:35](https://www.youtube.com/watch?v=lkgszkPnV8g&t=275s) 头号公敌:容器越界与 AddressSanitizer 在切入具体的新奇案例之前,Louis Brandy 强调必须首先坦承 Facebook 代码库中数量最多、破坏力最大的 Bug 根源:“如果不提及 Facebook 代码库中引发 Bug 的单项最大元凶,那在智识上就是不诚实的——它就是 `std::vector` 的下标操作符 `operator[]`。”他把 C 风格的数组下标索引与裸指针解引用归为同类,这几乎是所有代码库中引发问题最多的源头。 不过,Louis Brandy 将容器越界定性为本次演讲中“不予深入展开”的主题。原因在于它不是 C++ 自身设计缺陷引发的“自残式伤害(Self-inflicted wound)”,而是计算机科学在内存和数组访问上普遍面临的固有难题。要搞垮线上系统,最直截了当的方式就是不检查边界直接使用下标访问,或者循环遍历两个本应等长、实际却长度不一的动态数组。 针对这种越界缺陷,目前的防范技术路线主要包括: - **静态分析(Static Analysis)**:学术界和工业界对此研究已久,但要实现精准跨过程分析(Interprocedural analysis)的代价极其高昂。 - **现代抽象与算法(Range-based Algorithms)**:C++ 正逐步推广基于范围的循环与算法,将繁琐的手动下标迭代替换为封装良好的迭代器逻辑,很多时候能够把越界校验提升(Hoist)到循环体外部完成。 - **运行时强制边界检查**:如 Ada 语言所做的那样,在每次数组访问时强制执行边界检查,但这对追求极致性能的 C++ 系统会带来开销权衡。 - **动态插桩与 Fuzzing**:在 C++ 现有的工程生态下,最立竿见影的武器依然是编译器原生的 AddressSanitizer。 关于工具体系的防御价值,Louis Brandy 给出了一条坚决的建议:“如果你今天听完讲座什么都没记住,你唯独需要记住并带走的就是编译选项 `-fsanitize=address`。”他强调,结合模糊测试(Fuzzing)与 ASAN 是目前拦截数组越界与内存违规访问最强有力的工程防线。 ### [07:03](https://www.youtube.com/watch?v=lkgszkPnV8g&t=423s) 最隐蔽的初学者陷阱:std::map 的下标操作符 进入具体的 Bug 排行榜,Louis Brandy 按照程序员在职业生涯中遭遇问题的典型先后顺序,首先揭开了他心目中“全 C++ 语言中最阴险的初学者陷阱”——`std::map` 的下标操作符 `operator[]`。当他抛出这一提名时,台下观众席中立刻传来了心领神会的低语。 为了说明该问题的反直觉性,他列举了一段极简代码:先在 `std::map<std::string, int>` 中将键 `"hey"` 赋值为 `12`,随后在打印输出时发生拼写失误,误写成了一个不存在的键名。对于不熟悉 C++ 内部机制的程序员来说,他们通常会猜测程序会崩溃、抛出未捕获异常,或者打印出未初始化的垃圾内存值;在他们列出的所有可能行为中,实际发生的情况往往排在第三名开外: - 屏幕上直接静默地打印出数字 `0`。 - 程序非但没有报错,还悄无声息地向 Map 中新插入了一条键值对记录。 - 该语句执行完毕后,Map 的元素总数从 `1` 变成了 `2`。 Louis Brandy 感叹道:“如果你觉得这很正常、理所当然,那说明你写 C++ 的时间真的已经太长了。” ```mermaid flowchart LR accTitle: std::map operator bracket unexpected insertion accDescr: 使用 map 的下标操作符读取不存在的键时,会导致默认构造并静默插入条目。 subgraph Code["代码调用"] A["m['typo_key']"] end subgraph MapBehavior["std::map 内部执行"] B{"查找键是否存在?"} C["返回现有值引用"] D["默认构造值类型<br/>(Value Initialization 产生 0)"] E["插入新条目到红黑树"] end A --> B B -->|"存在"| C B -->|"不存在"| D D --> E E --> F["返回新增条目的值引用<br/>(导致 size 变为 2)"] ``` 在日常业务开发中,经验丰富的工程师早已习惯利用这一特性来编写极简的字符计数统计逻辑,例如直接通过 `++m[char]` 统计字符频次。相比于 Java 等需要显式判断是否存在并初始化变量的语言,C++ 的整型在下标访问时“不知为何就已经被赋上了零值”。然而,一旦初学者停下来追问底层细节——“为什么整型没做任何赋值操作就变成了 0?普通的裸 int 变量也会自动置 0 吗?”——资深开发者往往会陷入尴尬,不得不试图向对方解释复杂的“值初始化(Value Initialization)”规范。正如 Louis Brandy 调侃的那样,当你面对一个刚刚掉入下标陷阱的初学者时,你绝对不想在这个时候被迫开启关于 C++ 初始化规则的噩梦级长篇大论。 ### [09:29](https://www.youtube.com/watch?v=lkgszkPnV8g&t=569s) `std::map::operator[]` 与 const 正确性陷阱 在讲解如何防范新手踩坑时,团队通常会反复向初级程序员灌输“const 正确性”(const correctness)的原则,告诫大家只要坚持 const 正确就能避免大量 Bug。然而,当开发者尝试使用下标操作符 `[]` 从一个 `const std::map` 中读取数据时,编译器会直接报错拒编译,因为 `operator[]` 具备在键不存在时插入默认值的潜在修改行为。此时,新手往往不会去反思容器的数据访问语义,而是采取最直观的“修复”手段——直接把 `const` 关键字删掉。 ```mermaid flowchart TD accTitle: 编译错误误导初学者修改 const 的流程 accDescr: 展示初学者在遇到 map 方括号操作符报错时,误读编译器提示并删除 const 的全过程 subgraph CompilerFlow["编译器与报错"] A["读取 const map: map[key]"] --> B["编译失败: operator[] 要求非 const"] B --> C["GCC 报错: 'passing const... as this argument discards qualifiers'"] end subgraph DevReaction["新手开发者的反应"] C --> D["开发者字面理解: '传递 const 导致了错误'"] D --> E["开发者操作: 直接移除 const 修饰符"] E --> F["代码编译通过,隐藏深层缺陷"] end ``` Louis Brandy 分享了一个亲身经历:他曾指导一位刚入职 Facebook 的新人排查问题,看到对方写了类似的代码便询问为什么不加 `const`。对方委屈地表示“加了 `const` 就编不过”。当他们一起加上 `const` 并查看编译器报错时,终端赫然打印出:`error: passing const std::map... as this argument discards qualifiers`。Louis Brandy 尖锐地指出,资深 C++ 程序员能看懂这代表底层成员函数缺少 const 限定符,但对新手而言,报错信息字面上的意思就是“传递 const 出错了”,所以新人的逻辑非常简单直白:“既然传 const 会报错,那我删掉它不就解决了吗?” - 编译器报错详情:`error: passing 'const std::map<...>' as 'this' argument discards qualifiers`。 - 新手开发者的应对策略:“删掉 `const`,代码编译成功,即使导师强调过 const 正确性,但能跑就行。” ### [11:06](https://www.youtube.com/watch?v=lkgszkPnV8g&t=666s) 隐式插入默认值引发的生产重大事故 如果以为这种低级错误只会发生在新手身上,那就大错特错了。Louis Brandy 透露,据他所知,仅仅因为这一模式,Facebook 就发生过至少两次严重的线上全站事故(major site incidents),而且是由经验极其丰富的 C++ 团队写出来的代码引起的。在其中一个典型案例中,某个服务处理类(service handler)的构造函数接收了一个庞大的配置项字典(settings map),并将该参数赋值给类的成员变量,随后在日志打印语句中输出配置内容:“Server initialized with timeout...”。 ```mermaid flowchart LR accTitle: 下标操作符引发死锁连锁反应 accDescr: 说明配置字典缺失键导致超时被设置为 0 并引发无限超时的故障链条 A["配置 map 中缺少 'timeout' 键"] --> B["日志打印中使用 map['timeout']"] B --> C["隐式插入键并将 timeout 设为 0"] C --> D["套接字库将 0 解析为无限超时"] D --> E["下游服务挂起,所有工作线程全部卡死"] ``` 在某次代码重构后,传入该类的字典中恰好缺少了 `timeout` 这一项。极具讽刺意味的是,开发团队仅仅为了在打印日志时读取 `settings["timeout"]`,下标操作符就“贴心地”在 map 中就地插入了该键,并将其默认构造赋值为 `0`。在绝大多数底层网络套接字(socket)库中,超时时间设为 `0` 并不代表立即超时,而是代表“无限等待”。Louis Brandy 讽刺道,这真是一种绝妙的级联故障机制:“一个下游服务刚刚出现异常,当前服务的所有工作线程就立刻全部陷入无限超时等待中。” 针对这一陷阱,工具链团队目前几乎束手无策: - Louis Brandy 坦承:“It's actually the one we're the most powerless against. I have nothing to help you, I have nothing.”(面对这种新手陷阱,我们实际上是最无能为力的。我帮不了你们,我手里没有任何现成方案。) - 甚至有工程师严肃建议在公司内部代码规范中彻底禁用 `std::map::operator[]`。 - 现场观众建议使用 `.at()`,但 `.at()` 在键缺失时会抛出异常,在实际工程中存在其他性能与异常控制流方面的争议。 ### [12:56](https://www.youtube.com/watch?v=lkgszkPnV8g&t=776s) `findWithDefault` 与临时变量悬垂引用的深渊 其他高级语言通常采用类似 `getWithDefault(key, default_value)` 的模式:若键不存在则返回给定的默认回退值,且不会抛异常或向原 map 中插入脏数据。Louis Brandy 展示了 GHC Haskell 标准库中的实际实现 `findWithDefault`,盛赞其凭借模式匹配与类型推导写得优雅干净,并将其与 Facebook 核心开源库 Folly 中的 C++ 实现进行了对比。Folly 的模板函数需要处理大量的模板类型噪声,其基本逻辑是查找迭代器,若未到达 `end()` 则返回其对应的值,否则返回传入的默认值。 ```mermaid flowchart TD accTitle: 引用返回与悬垂引用的产生过程 accDescr: 展示将默认值作为临时对象以常引用返回时导致的悬垂引用 Bug A["调用: findWithDefault(map, key, std::string('default'))"] --> B["查找 key: 未在 map 中找到"] B --> C["函数返回对默认参数的 const 引用"] C --> D["当前语句执行完毕,传入的临时 string 对象被析构"] D --> E["调用方绑定的引用变为悬垂引用 (Dangling Reference)"] E --> F["线上访问野指针导致崩溃"] ``` 然而,这段极其直观的 Folly 模板代码在 C++ 程序员眼里却存在一个“深恶痛绝”的缺陷:为了通用安全,函数默认按值返回(return by value),这意味着在未命中或命中时都会发生一次对象的额外拷贝。许多注重极致性能的开发者试图将其重构成“返回 `const` 引用”(`const T&`),以消除多余拷贝。Louis Brandy 严肃警告:“This code is hopelessly broken. Do not copy and paste this code.”(这段代码从根本上就彻底坏掉了,千万不要复制粘贴这段代码。)因为在实际调用时,开发者几乎总是直接传递一个临时对象作为默认值(例如字面量构造的临时 `std::string`);一旦键未命中,函数返回的就是这个临时对象的常引用。当完整表达式执行完毕后,临时对象瞬间析构,调用方拿到手的便是一个悬垂引用(dangling reference),进而直接导致生产环境崩溃。 - 该将值返回改成引用返回的危险补丁在内部代码库中被提交了多达 4 次,其中 3 次在 Code Review 阶段被拦截,但有一次成功溜进主分支引发了线上事故。 - Louis Brandy 甚至开玩笑说需要开发一种“针对 Diff 的负向缓存”,只要检测到有人提交这个改动就自动拒掉。 - 这一问题属于 C++ 语言层面上将临时对象绑定到引用并逃逸传递的深层缺陷,很难仅依靠库技巧完全规避。 - 缓解该问题的利器是 AddressSanitizer(ASan),但在早期需要手动开启特定检测标志。当 Facebook 团队在 CI 中开启对应 ASan 标志后,当场抓出了几十个由临时变量悬垂引用引起的隐藏 Bug。 ### [17:08](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1028s) 中场休息:消灭 C++ 最恶名昭彰的 `volatile` 关键字 在结束第四个前置 Bug 的讨论后,Louis Brandy 带来了一段轻松的“中场休息”(intermission)。不同于那些屡禁不止的顽疾,这是一个团队最终成功打赢并彻底根除的历史 Bug 战役。这个 Bug 围绕着整个 C++ 中最糟糕、最容易被误解的关键字展开。当向现场观众发问谁是那个最烂关键字时,台下相继抛出了 `auto`、`goto` 等答案,随后有人喊出了正解:`volatile`。 在历史上,大量开发者误以为 `volatile` 具备跨线程同步的能力,常常用它来修饰多线程共享的布尔标志位,以为这样就能阻止编译器优化并安全地在线程间通信。Facebook 代码库中绝大多数对 `volatile` 的使用都是这种严重错误的并发尝试。由于服务器端开发并不涉及内存映射 I/O(MMIO)等真实的底层硬件寄存器场景,团队决定实施一个近乎“不讲道理”的代码审查规则:只要在代码中写下 `volatile`(除非在注释中),Linter 就会在代码提交页面(Diff)上自动喷出一整段密密麻麻的严厉说教文本。 ```mermaid flowchart TD accTitle: Facebook 针对 volatile 提交的 Linter 处理流程 accDescr: 展示 Linter 拦截 volatile 关键字并教育开发者的防御机制 A["开发者提交包含 volatile 的代码"] --> B{"是否写在注释中?"} B -- 是 --> C["放行通过"] B -- 否 --> D["Linter 报错: 自动喷出大段理论说教文本"] D --> E["开发者反驳: '我这样用了 20 年,阻止编译器优化'"] E --> F["工具团队介入辩论并指出并发模型漏洞"] F --> G["修改为 std::atomic,彻底在代码库清除该关键字"] ``` 这一激进的 Linter 规则在推行初期激怒了大量资深工程师,引发了连番论战。不少开发人员跳出来反驳:“我在这里用得很规范,我只有一个写线程修改 bool,另一个读线程去读取它,我只是不想让编译器把变量优化进寄存器,我们这么干已经写了 20 年代码了!”然而,无论经历多少次硬碰硬的争吵,基础架构与工具团队始终寸步不让,耐心指出单凭 `volatile` 无法提供内存屏障与原子性保障的事实。Louis Brandy 自豪地宣布,这一仗他们最终彻底打赢了,成功将不规范的 `volatile` 从庞大的 C++ 代码库中赶尽杀绝。 ### [19:17](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1157s) 2010 年代初的“volatile 战争”与语言抽象的演进 在回顾并发模式的变迁时,Louis Brandy 提到在 Facebook 工作了几年的人,大多已经不记得当年关于 `volatile` 的激烈争论了。“2010 年、2011 年的‘伟大 volatile 战争’,它们都已成为过去”,程序员们早已不再使用 `volatile` 来进行多线程同步。Brandy 认为停下来思考这背后的转变过程非常耐人寻味:当年究竟发生了什么才促成了这种转变? 事实证明,开发者当初之所以滥用 `volatile`,是因为他们真正需要的是原子标志(atomic flags)、互斥锁(mutexes)或者更高层级的并发抽象。一旦语言和标准库开始提供这些正确的工具,`volatile` 的滥用就悄然消失了。Facebook 既对老一辈工程师进行了重新培训,而新加入该语言的程序员也会自然而然地找到符合其需求的正确工具。Brandy 由此总结出一个有趣的规律:“事实证明,人们其实很想做某件事,并且会想方设法去实现它;如果你直接给他们提供一个现成的解决方案,他们就会去使用它。” ### [20:08](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1208s) C++ 中最令人胆寒的疑问:“`shared_ptr` 是线程安全的吗?” 第 5 个致命 Bug 模式源于 C++ 领域最令人头疼的问题。Brandy 直言不讳地指出:“这是整门 C++ 语言中最糟糕的问题,每当看到它,都会让我感到毛骨悚然。”在 Facebook 内部的讨论组中,这类求助的开场白往往如出一辙:某个团队遭遇了一个持续了整整一个月的间歇性崩溃,该问题线下极难复现,他们最终只能在生产环境中进行二分排查(bisect)——每次编译一个二进制文件,在生产机器上跑 6 个小时,如果崩溃就说明有问题,不崩就算过关。在漫长的三周排查后,他们终于锁定了产生问题的具体提交(diff),但团队看着改动目瞪口呆,完全看不出代码错在哪里。反复推敲后,他们得出了一个假设,并最终战战兢兢地问出了那个仅有三个词的问题:“`shared_ptr` 线程安全吗?” 现场观众中有人立即高呼“不安全”,也有人认为答案取决于具体场景。针对流传极广的“它和普通指针一样具有相同的线程安全性”这种说法,Brandy 评价其虽然技术上正确,但对排查问题毫无帮助。他在 Google 上搜索“is shared_ptr”,排在第一位的自动补全就是“is shared_ptr thread safe”;顺带一提,第三位是“is shared_ptr slow”(答案显然是否定的,若它出现在性能分析热点中,说明代码写错了),第四位是“is shared_ptr null”(答案是“是的,记得做空指针检查”)。针对这一核心问题,Brandy 给出了自己的定论: > “对于‘`shared_ptr` 是线程安全的吗’这个问题,我认为答案是‘不安全’。从技术细节上讲,这或许算句谎言;但我回答的其实是另一个问题——你表面上问的是 `shared_ptr` 是否线程安全,但你内心真正想问的其实是‘我的代码写对了吗?’,而一旦你必须开口问这个问题,答案就必然是否定的。” ### [22:38](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1358s) `shared_ptr` 的心智模型与直觉盲区 为了向中级开发者讲清这个问题,Brandy 建立了一个极简的心智模型(非具体底层实现细节,但足够抽象)。一个 `shared_ptr` 结构体本身在底层本质上是一对指针:一个指针指向共享的数据对象 $T$,另一个指针指向共享的引用计数(严格来说是包含删除器等信息的控制块 Control Block)。当进行拷贝构造或赋值时,程序仅仅是复制了这个包含双指针的结构体,使其指向相同的引用计数和相同的 $T$,并递增引用计数。 ```mermaid flowchart LR accTitle: shared_ptr 内存结构与线程安全性分布 accDescr: 图表展示了 shared ptr 内部双指针结构及其指向的共享对象 T 与控制块引用计数的线程安全情况 subgraph PtrInstance ["shared_ptr 实例结构(非线程安全)"] P1["对象指针 (T*)"] P2["控制块指针 (Control Block*)"] end subgraph Targets ["指向的目标(堆内存)"] T["共享对象 T<br>(未同步:需使用者自行加锁)"] CB["控制块 / 引用计数<br>(已同步:底层原子操作保障)"] end P1 -->|"访问/修改数据"| T P2 -->|"原子递增/递减"| CB ``` 基于这一模型,其线程安全性被清晰地切分为三个部分: 1. **共享对象 $T$**:任何涉及 $T$ 本身的读写都是未同步的。`shared_ptr` 不会对 $T$ 的并发访问提供任何保护,使用者必须自行加锁或同步。 2. **控制块(引用计数)**:`shared_ptr` 会全权维护控制块的安全,多个线程并发析构不同的 `shared_ptr` 副本时,引用计数的递减操作完全是原子的。 3. **`shared_ptr` 实例本身的读写**:这也是最常引发线上灾难的误区——许多人试图在没有外部同步的情况下,跨多个线程直接读写*同一个* `shared_ptr` 变量。这在并发下必然崩溃,其原因与在两个线程中无保护地同时读写一对普通裸指针完全一致。 Brandy 转向关注工程团队培训时的认知盲区。在实际工程实践中,新入职员工的直觉通常能在两点上保持正确:他们本能地知道 $T$ 是未同步的,也清楚控制块引用计数由标准库自动处理。然而,大家的直觉恰恰在自身结构上集体翻车:“人们总是不可遏制地想要在未同步的情况下并发读写同一个 `shared_ptr` 实例。”当 Brandy 在 Facebook 内部向同事验证这个现象并指出这一点时,对方当场恍然大悟:“原来就是这个导致的崩溃!我们代码里有个 Map 存了 `shared_ptr`,查了半天的崩溃根源就在这里!” ### [25:17](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1517s) 经典的退化版 RCU 故障模式及解决方案 在 Facebook 的服务架构中,这种由于对 `shared_ptr` 直觉错误引发的典型 Bug 反复上演,它通常表现为一种有缺陷且退化的 RCU(Read-Copy-Update)模式。在典型业务服务中,往往有一个后台更新线程和一个业务工作线程池: - 更新线程大约每隔 20 分钟唤醒一次,读取后端数据库,构建出最新数据在内存中的高效数据结构,并将其打包赋值给一个作为全局状态的 `shared_ptr`。 - 工作线程在处理每个进来的网络请求时,首先从该全局位置获取该 `shared_ptr` 的一份拷贝(增加引用计数),并在整个请求处理周期内稳定使用此快照,从而确保在请求期间不会受到外部并发修改的干扰。 这种架构具备很高的服务性能,因为更新操作成本极低且不阻塞处理流程(代价是需要容忍内存中同时存在多个瞬时版本的快照)。该方案的核心实现本来非常简单,开发者只需要用一把互斥锁(Mutex)去保护对那个共享中心 `shared_ptr` 的读取(原子获取)和写入(原子替换)即可。然而,由于开发者直觉上的松懈,许多人直接省略了对中心指针的互斥保护,最终陷入长达数周的排查地狱。 在故障诊断与标准演化方面,Brandy 补充了以下关键细节: - **诊断工具的局限**:ThreadSanitizer 能够准确捕捉到此类数据竞争;AddressSanitizer 虽然也会报错,但往往只是显示内存破坏(例如由于指针被并发破坏导致释放了错误的内存地址),这使得排查人员难以直接顺藤摸瓜定位到根因。 - **C++ 标准委员会的演进周折**:C++11 原本引入了针对 `shared_ptr` 的原子重载自由函数(如 `std::atomic_load` 等),理论上能解决问题,但大多数标准库实现并不完备或难以使用。后续标准委员会讨论了专门的 `atomic_shared_ptr`(以及类似 `std::atomic<std::shared_ptr<T>>` 的特化),并在多伦多会议(Toronto meeting)等场合历经反复表决与拉锯。由于并发工作组(SG1)讨论异常繁复,连参与 Evolution Working Group 的 Brandy 向 SG1 成员打听背景时,对方都表现得“双眼发直、精疲力竭”。 - **工程实用替代方案**:在业务代码中并不强求极致性能的无锁 `atomic_shared_ptr`,使用简单的互斥锁即可彻底解决问题;此外,委员会当时讨论的 Cell 抽象提议也是极佳的解决方案。 - **衍生额外陷阱(Bonus Bug)**:在业务代码中还普遍存在另一个令人难以理解的反模式——某函数明明返回了一个局部的 `shared_ptr` 右值,调用方却在同一行立即对其执行解引用(dereference)并绑定给一个底层对象的裸引用(`const T&`),导致临时 `shared_ptr` 析构后引发悬挂引用(dangling reference)。 ### [28:43](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1723s) 悬空引用与失去引用计数保护的 Bug Louis Brandy 正在收尾前一个关于引用与智能指针的陷阱分析。当代码取得一个对象的引用(例如 `ref`)后,如果不慎通过解引用操作提前将底层的 `std::shared_ptr` 销毁,接下来调用 `ref.boom()` 就会引发未定义行为。 ```mermaid flowchart TD accTitle: Shared Pointer Lifetime Mismatch accDescr: Dereferencing and destroying a shared pointer leaves the reference unprotected without reference count. A["持有 std::shared_ptr"] --> B["获取内部对象的引用 ref"] B --> C["shared_ptr 被提前销毁/重置"] C --> D["ref 失去引用计数保护成为悬空引用"] D --> E["调用 ref.boom() 导致内存崩溃"] ``` 在 `std::shared_ptr` 被销毁的那一刻,代码实际上已经失去了引用计数带来的生存周期安全保障,后续所有操作都是在没有任何引用计数保护的裸指针语义下运行。Louis Brandy 强调:“So all your code is now running without a reference count protecting you.”这类 Bug 在 Facebook 的生产环境中屡见不鲜,不过幸运的是,AddressSanitizer(ASan)非常擅长且能极其容易地捕捉到这种悬空引用问题。 --- ### [29:10](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1750s) 代码审查互动:`std::string(foo);` 究竟能否编译? 紧接着,Louis Brandy 抛出了整场演讲中他最喜欢也是最具欺骗性的 Bug 模式——第六号 Bug。为了让现场观众身临其境地感受,他把这一节设计成了互动式代码审查(Code Review),并在大屏幕上展示了一段极短的代码,只包含两行核心声明: ```cpp #include <string> void f() { std::string(foo); } ``` Louis Brandy 向全场提出了经典的“C++ 猜谜游戏”: 1. 这段代码能通过编译吗? 2. 如果不能编译,你期望看到什么样的报错信息? 现场只有大约 10% 的人举手认为这段代码可以编译。然而,举手的人群中赫然坐着 C++ 标准委员会的核心人物、时任 C++ 标准编辑 Richard Smith。Louis Brandy 调侃道:“The editor of the C++ standard says this code compiles. Who's changed their mind?” 不出所料,Richard Smith 是对的,全场绝大多数资深 C++ 工程师全猜错了——这段代码完全可以通过编译。Louis Brandy 笑着建议正在看视频的观众暂停画面,先认真想想它为什么能编译,然后再从哲学层面思考一下人生:“like what are we doing, why are we here? I feel like I'm wasting my life a little.” 其底层逻辑源于 C++ 语法的歧义解析规则:在这一部分标准中,凡是长得像声明(declaration)的语句,无论它可能原本意图是什么,都会被无条件视作声明。因此: ```cpp std::string(foo); // 等价于 std::string foo; ``` 包围在标识符外部的多余圆括号 `()` 完全是合法的可选语法。在 Compiler Explorer(Godbolt)上不开启优化的汇编输出清晰显示:程序仅仅是默认构造了一个名为 `foo` 的空字符串,随后在其生命周期结束时将其析构。 --- ### [31:42](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1902s) 伪装成锁的变量声明:`std::unique_lock` 的静默失效 Louis Brandy 指出,如果仅仅停留在 `std::string(foo);`,这只是个微不足道的冷门语法谜题(Trivia)。但一旦这种语法特性与 RAII 锁机制相碰撞,就会演变成整个 C++ 体系中最常见、最恐怖的并发 Bug 之一。 在一个需要内部线程同步的多线程类中,开发者经常会在修改状态的方法开头尝试获取锁。很多工程师会下意识地写出以下代码: ```cpp class ThreadSafeObject { std::mutex my_mutex; public: void update() { std::unique_lock(my_mutex); // 极度隐蔽的致命 Bug! // 临界区业务代码... } }; ``` 现场只有半数人一眼看出了破绽。写下这行代码的人,原本意图是像 `std::lock_guard` 一样,构造一个匿名的锁守卫对象,通过构造函数锁住成员变量 `my_mutex`,并在作用域结束时自动析构解锁。 ```mermaid flowchart TD accTitle: Unique Lock Syntax Shadowing Bug accDescr: Attempting to instantiate an unnamed lock guard inadvertently declares a shadowing variable without locking. A["开发者意图: 匿名锁守卫 RAII 加锁"] --> B["书写 std::unique_lock (my_mutex);"] B --> C["编译器视作声明: 声明一个名为 my_mutex 的 local unique_lock"] C --> D["新变量遮蔽(Shadow)了类的成员互斥量"] D --> E["没有调用成员互斥量的 lock(),临界区完全处于裸奔状态"] ``` 现实机制截然相反: - 该语句被解析为:声明了一个类型为 `std::unique_lock`、变量名叫 `my_mutex` 的局部变量。 - 这个局部变量无声无息地**遮蔽(Shadow)**了类原本的 `my_mutex` 成员变量。 - 整个过程根本**没有任何加锁行为发生**。临界区处于完全无保护的并发数据竞争(Data Race)状态。 正确的修复方式众人皆知:必须给锁守卫赋予一个独立的变量名(例如 `std::unique_lock<std::mutex> g(my_mutex);`)。Louis Brandy 呼吁现场所有维护现代 C++ 代码库的工程师回去立刻执行针对 `std::unique_lock` 模式的全局代码检索。他在 Facebook 庞大的代码库中搜索后,果然揪出了两起实际存在的 Bug(形如 `std::unique_lock (mtx);`)。令人深思的是,Facebook 内部早就部署了专门检测这种模式的 Linter 静态检查规则,却依然有两处漏网之鱼潜伏在主干代码中。 --- ### [34:34](https://www.youtube.com/watch?v=lkgszkPnV8g&t=2074s) 史上最折磨人的 C++ 歧义语法连环测验 为了彻底理清圆括号声明与构造函数的边界,Louis Brandy 现场组织了一场被称为“史上最恶毒”的 C++ 连环测试题: | 代码片段 | 是否通过编译 | 编译器实际解析与底层原因 | |:--- |:---: |:--- | | `std::string(foo);`<br>`std::string("bar");` | **通过** | 第一行是变量声明(名为 `foo` 的局部字符串);第二行则是用 `const char*` 构造的一个临时 `std::string` 对象。 | | `std::string(foo);`<br>`std::string(foo);` | **失败** | **重定义错误(Redefinition error)**。由于语法歧义原则规定“只要长得像声明就是声明”,第二行被视为在同一作用域内第二次声明名为 `foo` 的变量。 | | `std::string(foo);`<br>`std::string((foo));` | **通过** | 第一行声明变量 `foo`;第二行通过额外的括号打破了声明匹配,被解析为表达式,调用拷贝构造函数生成一个以 `foo` 为参数的临时对象。 | | `std::string(foo);`<br>`{ std::string(foo); }` | **通过** | 第二行位于嵌套的花括号作用域内部,属于在内层作用域中重新声明名为 `foo` 的变量,合法遮蔽(Shadow)了外层的 `foo`。 | 针对第二道“同名写两次”的代码,现场有观众提出疑问:第二行难道不能解释为利用第一个 `foo` 进行拷贝构造生成临时对象吗?Louis Brandy 回应:“Because the rule says if it's ambiguous with a declaration, then it is a declaration, period. And that's why you get a redefinition error.” 只要语法上能匹配声明,编译器就绝不会将其解析为表达式。 --- ### [37:14](https://www.youtube.com/watch?v=lkgszkPnV8g&t=2234s) 漏洞根源剖析:RAII 盲区与默认构造函数的致命隐患 Louis Brandy 深入剖析了为何 `std::unique_lock` 会成为这种恶性 Bug 的重灾区。他指出,这背后是两个特性的“恶毒结合”: 1. **RAII 对象的特殊性**:普通类型的变量如果不小心漏写变量名,后续代码一旦试图调用它的接口,编译器立刻就会因为找不到变量而报错。但 RAII 类型完全不同——它们的大部分甚至全部功能仅仅依赖于“声明和销毁”本身,开发者在后续代码中压根不需要引用它的名字,这导致开发者漏给变量命名时毫无感知。 2. **默认构造函数的存在**:这种声明语法之所以能够编译成立,核心前提是该类型**具备默认构造函数**。`std::unique_lock` 恰好提供了无需参数的默认构造函数,因此 `std::unique_lock (mtx);` 实际上调用的是默认构造函数来声明变量 `mtx`。 相比之下,标准库中的 `std::lock_guard` 并没有默认构造函数。如果将上述 Bug 代码中的 `std::unique_lock` 替换为 `std::lock_guard`,编译器会直接报错而无法通过编译,因此标准 `lock_guard` 天然免疫此问题。 然而,像 Facebook 这样拥有深厚历史的大型工程体系中,过去十几年间内部积累并重复造了数十个不同版本的自定义 `lock_guard` 实现。由于早期很多编写基础库的工程师并未意识到这个隐秘陷阱,Facebook 团队在审计自定义锁守卫时,发现了数十起甚至上百起因为自定义 `lock_guard` 提供了默认构造函数而导致的严重同类并发隐患。Louis Brandy 疾呼:“So if you have a lock_guard, run, don't walk, to see if it has a default constructor.” 关于 C++17 引入的新工具 `std::scoped_lock` 是否存在类似问题,Louis Brandy 表示情况更为复杂。在演讲前一天他放入 Compiler Explorer 测试时,Clang 直接拒绝编译此类写法,而 GCC 却能够编译通过,各家编译器的解析实现尚存在分歧。 ### [39:12](https://www.youtube.com/watch?v=lkgszkPnV8g&t=2352s) RAII 默认构造函数的阴影陷阱与编译器告警缓解方案 针对前面分析的锁保护与变量声明缺陷,Louis Brandy 给出了最终的总结论(TL;DR):如果一个 RAII 类型自带默认构造函数,代码库就会陷入极其危险的境地。开发者的本意通常是想封装某个底层互斥量,却极易由于疏忽忘记给包装对象命名,直接写成了把对象类型当成了声明变量,从而把外部作用域同名的对象给阴影遮蔽(shadow)了。 在编译器缓解手段方面,GCC 和 Clang 长期以来提供了 `-Wshadow` 选项。如果开启该选项,它确实能够百分之百抓住这种内层作用域变量遮蔽外层变量的错误。然而用过 `-Wshadow` 的工程师都知道,它的告警极其吹毛求疵(pedantic),产生的噪声铺天盖地,逼得团队最终只能将其全局关闭;若强行去修那些无关紧要的告警,代码甚至会变得更难看。 为了在信号与噪声之间取得平衡,编译器衍生出了一些混合告警标志: - `-Wshadow-compatible`:仅在被遮蔽的两个变量类型兼容时才触发告警。 - `-Wshadow-compatible-local`:进一步收紧限制,仅当两个兼容类型的变量同属于同一个局部函数内部时才告警。 Louis Brandy 强烈建议大家至少在工程中开启 `-Wshadow-compatible-local`。尽管打开它能揪出一大堆即便没报错也极其“代码恶臭”(smelly)的写法——例如外层循环用变量 `i`,内层一堆循环也跟着用 `i`——但遗憾的是,它**无法**捕获本演讲所讨论的这个未命名 RAII 阴影缺陷;唯有最严格、噪声最大的完整版 `-Wshadow` 才能发现。 ```mermaid flowchart TD accTitle: 变量阴影检测与告警范围对比 accDescr: 对比编译器原生告警选项与定制化静态分析工具在捕获未命名锁缺陷时的能力差异 subgraph CompilerWarnings["编译器原生告警 (-Wshadow 系列)"] WAll["-Wshadow<br/>能抓出此 bug,但噪声极大、过于吹毛求疵,常被全局关闭"] WLocal["-Wshadow-compatible-local<br/>噪声低,能抓出循环变量复用等代码恶臭,但无法抓出本 bug"] end subgraph Linters["定制静态分析与拓展工具"] ClangTidy["clang-tidy / 自研扩展 linter<br/>直接检查 '未命名的 lock_guard' 与特定语义规则,高信噪比捕获"] end CompilerWarnings -->|"权衡两难"| Linters ``` ### [41:08](https://www.youtube.com/watch?v=lkgszkPnV8g&t=2468s) 未命名锁与临时对象寿命陷阱:借助扩展 Linter 根除 除了变量阴影声明问题,还有一类紧密相关的常见恶性缺陷:本该在整个作用域内持续持有资源的 RAII 临时对象,因为少写了变量名,在单行代码执行完毕后就“立即眨眼间消失”(immediately wink out of existence)。这类瞬时析构的临时锁是引发并发竞态的罪魁祸首。Louis Brandy 指出,未来 C++ 如果能针对构造函数引入类似 `[[nodiscard]]` 的属性,或许有助于在语言层面上解决临时对象被意外丢弃的痛点。 但在目前,业界许多常规编译器和静态工具并不默认拦截未命名的 `lock_guard`。如果团队重度使用带默认构造函数的守卫或是 `std::unique_lock`,代码库中几乎必然潜藏着大量这种瞬时失效的假锁。针对此类场景,最切实有效的防线是基于 Clang 的可扩展静态检查器(例如 clang-tidy)。在 Facebook 内部,团队开发了自定义扩展规则,能精准拦截任何遮蔽外部互斥量的 `lock_guard`,用高信噪比的规则直接掐断这类隐患。 ### [42:03](https://www.youtube.com/watch?v=lkgszkPnV8g&t=2523s) 总结:“C++ 很难”,工具是最好的教学武器 在谈及演进与维护的感悟时,Louis Brandy 直言不讳地指出:“C++ is hard.”(C++ 真的很难)。这或许是一种主观的价值判断,但整个社区应当勇敢承认这一现实。在 Facebook,往往是技术极强的顶级 C++ 专家连续排查整整一周,最后才顺藤摸瓜发现崩溃的原因仅仅是 `lock_guard` 后面手滑少敲了一个字符 `g`。耗费数天心力、承受巨大的线上排障折磨,最后修复的补丁居然只有一个字符,这种打击让人极为挫败。 在培养团队与防止犯错方面,Louis Brandy 强调:“工具是我们最好的教学武器。”办技术讲座、开设内部培训课当然很有价值,他也希望能把每个 Facebook 工程师都抓去上一遍课,把所有不该写的缺陷全讲一遍,但现实中这种单向灌输的效果往往极差。向开发者指明错误的反馈回路越短,教育效果就越好: 1. **IDE 实时告警**:刚刚写出 bug 界面立即飘红,这是最完美、最极致的体验。 2. **编译器报错**:编译期直接拦截,体验同样极其出色。 3. **代码评审(Code Review)异步检查**:在 diff 提交后,重量级静态分析工具自动跑完并在变更集上留言标出确定性 bug,依然相当不错。 4. **两日后的静态分析邮件**:提交代码两天后,机房的超级计算机完成了跨过程静态分析并给你发了一封邮件,虽然不够敏捷,但也还能接受。 5. **两年后的技术复盘大会**:在某人花了一周痛苦排查 bug 的整整两年后,在大会演讲里把原理讲一遍,对于预防那个最初的错误而言基本毫无用处。 在内部工程落地中,除了他们戏谑命名为“how to even”(玩梗自流行语 you can't even)的 Clang 自扩展 linter,Address Sanitizer (ASan) 同样立下了汗马功劳。Louis Brandy 调侃道,虽然他无意把 Address Sanitizer 当作这场演讲中包治百病的灵药,但现实就是如此神奇。他对台下的开发者呼吁:如果在 GCC 或 Clang 环境下写代码却从未开过 ASan,现在就应该起身离席,立刻回到工位把 ASan 配上;跑一遍测试用例,你绝对能在代码库里当场挖出十来个隐藏 bug。他笑着建议:“在找老板谈加薪之前千万别声张,等加薪到手了,再去把 Address Sanitizer 教给你的团队。” ### [45:18](https://www.youtube.com/watch?v=lkgszkPnV8g&t=2718s) 问答环节:落选的常见缺陷、异步生命周期与函数返回值陷阱 演讲结束后进入现场提问环节,现场观众与 Louis Brandy 展开了多轮深入互动: - **关于落选本次演讲的 Bug 候选(45:19)**: 有观众询问有哪些 bug 没有入选最终的演讲。Louis Brandy 透露,许多人提名了“不必要的拷贝”,但他开玩笑说,恰恰是因为大家对拷贝厌恶到了极点,到处强行传引用,才导致了他在前面章节中大谈特谈的诸多致命生命周期悬空 bug,因此“过度拷贝”没被收录。 另一个高票提名但没能放进幻灯片的核心问题是**异步编程与生命周期(Asynchronous programming and lifetimes)**。Facebook 内部重度依赖 Folly Futures 和协程/纤程(fibers),异步调用链中的对象生命周期管理极度复杂。许多其他拥有成熟异步框架的语言通常自带垃圾回收机制(GC),因而能够轻松抹平生命周期问题,但在现代 C++ 中缺乏 GC 使得异步生命周期平衡极其棘手。 - **关于针对 `get_default` 等危险 API 的防御措施(46:21)**: 有观众追问,针对演讲前半段提到的因 `get_default` 返回临时对象引用而导致悬空的经典 diff,为什么不直接在代码里写上一大段注释提醒大家不要碰?Louis Brandy 解释说,他们最终的实际修复方案是为该函数提供了一个接收右值参数的重载版本,并显式将其声明为 `= delete`,直接在编译阶段禁止传入临时右值,随后才在旁边附上了大段警示注释。他坦承这里隐瞒了一个复杂的历史细节:Facebook 代码库里不止一个 `get_default`;Folly 库中的版本虽然修复了,但 STL 工具库中另一份年久失修的老旧实现由于疏漏未被及时拦截,最终还是偷偷溜进了代码库。 - **关于 `operator[]` 返回 `std::optional` 与 C++17 强制拷贝消除(47:07)**: 观众询问是否应该让 `map` 的下标操作符 `operator[]` 返回值的 `std::optional`(类似于 Haskell 返回 Maybe 类型),从而拯救悬空引用。现场其他参会者随即指出标准库中的 `std::optional` 并不支持存放引用(`optional<T&>`)。 该观众进一步追问,C++17 规范中引入的“强制拷贝消除”(Mandatory Copy Elision)究竟是会简化还是加剧人们对于过度拷贝的担忧?Louis Brandy 表示,强制拷贝消除总体上肯定是件好事,大家对拷贝开销保持警惕也是合理的。但其局限在于,并非所有接口都能直接利用这一机制——尤其是那些接收引用并试图返回引用的函数,极易将悬空引用暗中走私出来(smuggle a reference out),这种语言机制本身的生命周期隐患仍然独立存在且极为棘手。 ### [49:04](https://www.youtube.com/watch?v=lkgszkPnV8g&t=2944s) 复制消除与 Facebook 内部 Clang Linter 的开源考量 演讲进入最后的问答环节。有观众就前面的复制与移动语义讨论提出延伸疑问:如果在语言层面引入更复杂的空间优化机制,是否能解决此类复制问题?Louis Brandy 结合之前提到的 `get_default` 例子解释道,在这种情况下开发者根本别无选择,因为数据源可能不止一个,编译器无法直接将复制消除(Copy Elision);如果目标数据在 map 里,就必须老老实实拷贝出来。不过他也承认观众说的整体方向是合理的:“在平均情况下你可能是对的,这会让大家不那么对拷贝抱有偏执的心态。”(“So in that case, it won't help, but on average you're probably right. It'll make it much less likely for people to be paranoid about copies.”) 随后,观众席中的 Dmitri 提问,Facebook 内部构建的这些基于 Clang 的 linter 工具看起来极其有用,团队是否有计划将其开源。Louis Brandy 坦言这背后是一段相当长的故事:当年 Facebook 开发内部 linter 框架时,可用的 `clang-tidy` 尚未成熟,因此团队实际上自己造了一套类似但更简易的轮子。现在团队内部一直在深入讨论:究竟是直接把这套自研工具开源(但这可能会导致生态分裂),还是应该把所有自定义检查规则移植并重写为标准的 `clang-tidy` 规则?他总结道:“我想把东西开源出来,我们应该开源所有这些工具,我们平时也经常这么做;但我们得先把这个问题想清楚,这才是真正的症结所在。” - `get_default` 的语义决定了无法强制进行复制消除,因为存在两个来源(默认值或容器内的值),容器内的数据只能拷贝。 - 内部 linter 本质上是一个早期的、功能更简化的 `clang-tidy` 替代品,目前直接开源的实际价值有限。 - 团队正权衡是否将自研的检测规则重写并合并至上游的 `clang-tidy`。 ### [50:17](https://www.youtube.com/watch?v=lkgszkPnV8g&t=3017s) 性能优化的尺度与 `std::scoped_lock` 的潜在缺陷 一位观众站起来给全场泼了盆冷水,指出在规模差异面前大家的焦虑往往放错了地方:Facebook 这种体量哪怕多拷贝 100 字节,可能都会带来一百万美元的真金白银损失,而普通团队拷贝一下的成本几乎为零,大家手头有远比“多拷一次字符串”大得多的架构问题要解决。Louis Brandy 笑着表示赞同,不过他也指出了作为底层库作者的无奈现实:“如果你是一个库的作者,你的用户确实会在意这些。虽然即便那些在意的用户,很多时候也往往在乎得过头了——每次我都想说:‘不,在你的性能分析(profile)里根本看不到这块,赶紧走开吧。’但这个观点确实很中肯。” 最后,Tony 提问 Louis 是否向标准委员会反映过 `std::scoped_lock` 构造函数的类似问题。Louis 回应称自己曾和 SG1(并发工作组)讨论过,对方建议提交缺陷报告(DR),但 Tony 提醒他应该去找 LEWG(库演进工作组)。谈及 `scoped_lock` 是否真的存在类似陷阱,Louis 表示自己曾以为它确实受影响,但后来发现其交互极其复杂:“因为它是变长参数模板(Variadic Template),现在又有了类模板实参推导(CTAD),所以非常复杂,我其实不确定它到底会不会受到这种行为的影响。”台下观众随即起哄建议他现场抓标准委员会的专家请教,Louis 幽默地调侃了一句在场的 C++ 标准委员会核心成员 Richard Smith:“可怜的 Richard,他来之前根本不知道自己进的是哪场演讲。”演讲在全场掌声中正式落下帷幕。 - 观众观点:微观性能优化(多拷贝几次小数据)在极端规模下具有巨大经济意义,但在常规业务项目中往往并非主要矛盾。 - `std::scoped_lock` 结合了可变参数模板(variadic templates)与 C++17 的类模板实参推导(CTAD),其构造函数边缘行为极为微妙,是否需要提交 DR 仍待进一步确认。 ## Notable quotes > [20:08](https://www.youtube.com/watch?v=lkgszkPnV8g&t=1208s) 你表面上问的是 shared_ptr 是否线程安全,但你内心真正想问的其实是‘我的代码写对了吗?’,而一旦你必须开口问这个问题,答案就必然是否定的。 — Louis Brandy > [04:35](https://www.youtube.com/watch?v=lkgszkPnV8g&t=275s) 如果你今天听完讲座什么都没记住,你唯独需要记住并带走的就是编译选项 -fsanitize=address。 — Louis Brandy > [42:03](https://www.youtube.com/watch?v=lkgszkPnV8g&t=2523s) 工具是我们最好的教学武器。 — Louis Brandy ## Mentioned Louis Brandy, Facebook, PHP, Hack, HHVM, Python, Java, folly, AddressSanitizer, Linux, GCC, Clang, Ada, GHC Haskell, ThreadSanitizer, Richard Smith, Compiler Explorer, clang-tidy, Dmitri, Tony ## Video description http://CppCon.org — Presentation Slides, PDFs, Source Code and other presenter materials are available at: https://github.com/CppCon/CppCon2017 — I've spent the last few years watching Facebook's C++ codebase grow by several orders of magnitude. Despite constantly improving abstractions, constantly improving tooling, frequent internal courses, and ongoing internal discussion, there are bug-patterns we simply cannot stop from being reintroduced into our code. My hope is to show some of the most common (and infamous) bugs in our history, and the surprising complexity that arises in some apparently simple situations. This talk serves the dual purpose of educating the intermediate (and perhaps the occasional advanced) C++ programmer about some really nasty common pitfalls, as well as serves as a plea to experts to help further improve the language, libraries, and best practices to help educate and eradicate some of these problematic patterns. — Louis Brandy: Engineering Director, Facebook My team is responsible for the overall health of the Facebook C++ codebase, both the tools and the libraries. We work on: compilers, static/dynamic analysis, linters, large scale changes, and the core libraries. — Videos Filmed & Edited by Bash Films: http://www.BashFilms.com Work at Hudson River Trading (HRT): https://tinyurl.com/safxfctf --- Videos Filmed & Edited by Bash Films: http://www.BashFilms.com [Source](https://www.youtube.com/watch?v=lkgszkPnV8g)

Source quotes

[20:08] Louis Brandy: 你表面上问的是 shared_ptr 是否线程安全,但你内心真正想问的其实是‘我的代码写对了吗?’,而一旦你必须开口问这个问题,答案就必然是否定的。 [4:35] Louis Brandy: 如果你今天听完讲座什么都没记住,你唯独需要记住并带走的就是编译选项 -fsanitize=address。 [42:03] Louis Brandy: 工具是我们最好的教学武器。

Source quotes

[4:35] Louis Brandy: If you take nothing away from today, what you need to take away is -fsanitize=address. [50:17] Audience member: When Facebook copies 100 bytes too many, it costs them one million dollars. Me and you, it costs us zero. You have so much bigger problems than just copying a string one too many times. [37:14] Louis Brandy: So if you have a lock_guard, run, don't walk, to see if it has a default constructor.

Original transcript

[0:00] All right, it's 1:30, it's time. [0:03] Thank you everyone for coming back from lunch through rain. [0:06] I'm here today to talk about bugs. [0:09] My name is Louis Brandy, I work at Facebook. [0:11] I have brought to you a selection of bugs. [0:16] I want to give a tiny bit of context here before, [0:19] just to give you some background on why it is [0:22] I have opinions on recurring bug patterns. [0:26] One of the questions I get a lot, [0:27] I've been getting since I've come to CppCon, [0:29] is does Facebook actually have a lot of C++? [0:31] Like, don't you guys just use PHP? [0:34] It turns out we have an enormous amount of C++. [0:37] We've actually had enormous amounts [0:38] from effectively the beginning. [0:39] Facebook infra has been built on [0:42] C++ effectively from the beginning. [0:45] It's the second most used language [0:47] behind our PHP hack codebase, [0:48] and our PHP hack codebase runs on something [0:50] called HHVM which is a C++ process, [0:52] which sort of lives in our repository. [0:56] There's other languages and infra at Facebook. [0:57] Python, notably, is definitely second, [0:59] Java, third, and then [1:01] everyone's favorite language, et cetera. [1:05] There's actually quite a long tail of languages. [1:08] Let's see Clicker. [1:10] So our team is effectively the core C++ group at Facebook. [1:15] There are two groups in this group, [1:18] two logical sections. [1:19] One is very tool-focused, one is very codebase focused. [1:23] So the tool part of our team works a lot on things [1:26] like the compilers, the sanitizers, [1:28] the build systems, and so forth like that. [1:30] The code-based part of our team [1:31] deals a lot with the core libraries, [1:33] folly, we literally have a directory called common, right. [1:39] And so because of these two things, [1:40] like the sanitizer is one half, [1:41] the core library is another half, [1:42] we're often in escalation of point of both. [1:45] Me personally, I'm involved often [1:46] in our site event review meeting. [1:49] So when something crashes the site, [1:52] especially if it involves a C++ bug, [1:53] I see it. [1:55] So I have a very rich data source. [1:57] Sometimes it's crashed the site, [1:59] sometimes it's, hey, is this [2:01] an Address Sanitizer false positive? [2:03] It's like, no, no it's not, [2:05] you have a bug in your code [2:06] and it's subtle and it's here. [2:09] And so this is my data source for this talk. [2:12] So obviously my agenda is bugs. [2:13] I brought some bugs with me. [2:14] I had some criteria for the bugs I wanted to show you. [2:16] I want them to be scary, so like [2:18] a syntax error or whatever is not interesting. [2:21] Okay, so these are things that are scary. [2:22] I want them to be common and recurring. [2:25] So these are things that keep happening, [2:26] over and over, despite our best efforts. [2:29] They're also not the list of [2:30] the craziest thing I've ever seen. [2:32] Okay, that's a different talk. [2:34] And they're concrete, so I have bugs that fit on slides. [2:38] Like I'm going to show you C++ that is broken. [2:40] So it's not going to be like libraries [2:42] that have bad ownership models cause lots of bugs, [2:44] which is true, they do, [2:46] but that's abstract and not super interesting. [2:51] I'm going to talk about mitigation, [2:53] and all the tools we have. [2:55] I didn't intend for this to become [2:57] a recruiting event for ASAN. [2:59] But it turns out that ASAN is actually highly impactful [3:03] on almost every bug we're going to talk about today. [3:04] Almost every bug. [3:06] So we'll talk about ASAN a fair bit, [3:07] but also all the other tools. [3:10] And, just as fair warning, [3:12] especially since there's going to be [3:13] a lot of talk about sanitizers here, [3:15] gcc/clang Linux bias from me, [3:17] especially in the tooling part. [3:18] Those of you that use other tool chains, [3:20] hopefully have equivalent technologies. [3:22] If not, you should get some. [3:24] Okay. [3:26] All right, when you give a talk about bugs, [3:28] it's actually kind of terrifying, [3:30] because there's two different [3:31] groups of people you might be talking to. [3:33] One is audience number one, [3:35] I show you something you have never seen before, okay? [3:39] I save you from a terrible day, with any luck, [3:42] like if I'm interesting enough [3:42] for you to be paying attention. [3:44] Right, so I'm gonna show, I'm hoping [3:46] I'll show at least everybody [3:47] one thing that they've never seen before. [3:49] If you've seen it, you forgot, [3:50] and maybe we'll flesh it out a bit. [3:52] For some of you, like if you're a student [3:54] or something, maybe I'm going to show a bunch [3:55] of stuff you've never seen before, [3:56] and you might actually, [3:57] I might save you like a week, [3:58] the worst week of your career. [4:01] Audience number two, [4:02] I'm going to show you a bug you've seen a hundred times. [4:05] Almost always you see other people making, [4:07] you're the one in the code reviewing [4:08] whoa, whoa, whoa, whoa, whoa, [4:10] there's something you don't understand here. [4:11] And of course if you're in [4:12] audience number two and you're an expert, [4:14] you never make these mistakes yourself. [4:16] Never, never, never. [4:17] This never happens. [4:20] But you and I are having a slightly different conversation. [4:22] So our conversation is why do these keep happening? [4:26] What can we do about it? [4:27] Can we fix the language, can we make the tools better, [4:29] can we do education better, right? [4:31] But what can we do about it [4:32] to make some of this stuff go away? [4:35] All right, so enough gibberish. [4:36] Enough philosophy. [4:38] It would be intellectually dishonest [4:40] to not mention the single biggest cause [4:42] of bugs at Facebook, okay. [4:44] It is the square bracket operator of a vector, okay? [4:48] I put a bunch of stuff in this bucket [4:49] because frankly C index operator is the same thing [4:53] and frankly pointer dereferencing is roughly the same thing. [4:57] This is probably the single biggest cause [4:58] of problems in probably every codebase. [5:01] Right, it's the most important bug most of us have. [5:03] But for today's purposes, [5:05] I'm going to declare this uninteresting. [5:08] Only for today's purposes, okay. [5:10] And the reason I declare it uninteresting is [5:13] it's not really C++ self-inflicted wound. [5:15] We, computing has this problem, in general. [5:19] Every language is built or not built around it. [5:24] And so I think this is actually [5:26] a hard inherent problem in what we do. [5:29] Most of the rest of things I'm [5:30] going to talk about are not really inherent. [5:31] They were self-inflicted. [5:35] So yeah, if you wanna crash this site, [5:37] don't check the bounds of your vector. [5:39] Start indexing into it. [5:41] You'll definitely crash the site eventually. [5:42] Or better yet, integrate through [5:43] two vectors and assume they're the same length. [5:48] So mitigation. [5:49] So let's, we'll practice now, we'll practice mitigation. [5:51] I've declared this uninteresting, [5:52] but let's talk about it. [5:54] Static analysis has been studying [5:55] this problem for a really really really long time. [5:57] It's sort of the queen mother [5:58] of hard static analysis problems. [6:00] They've made a lot of progress, [6:02] but the good static analysis to find this is expensive. [6:05] Interprocedural, et cetera, et cetera. [6:08] A lot of languages, and C++ is headed in this way, [6:11] change the abstractions. [6:12] You don't wanna do for loops, [6:13] you wanna do range-based algorithms. [6:15] If you do range-based algorithms, [6:16] often times you can hoist [6:18] any bounds checking outside the loop. [6:21] And of course you can do things that run time, [6:23] you can literally bounce check every single array access. [6:26] This is what, like, Ada does, [6:27] and has done for a really long time for safety. [6:31] But as far as I'm concerned, at least in C++, [6:33] your best chance of finding these is Address Sanitizer. [6:36] Address Sanitizer was built for this. [6:38] If you really really really don't want [6:41] out of bounds problems in your code, [6:43] you need to have fuzzing with Address Sanitizer. [6:45] I'm not going to talk about it, [6:46] there's many talks to go talk about this particular problem. [6:48] But if you take nothing away from today, [6:50] what you need to take away is fsanitize=address. [6:54] Clang and GCC, again, specific, [6:56] but Address Sanitizer is probably [6:58] the most important thing that's happened, [6:59] at least in our tool set, in a long time. [7:02] All right. [7:03] So, bug number two, the greatest newbie trap, [7:06] in my opinion, I'm nominating, in all of C++. [7:12] I'm curious, by the way, before I tell you what it is, [7:14] if you have a personal nomination [7:15] and you think you've got me beat, come find me after. [7:17] I want to hear what you think is [7:18] the greatest newbie trap, in all of C++. [7:21] In my opinion, the greatest newbie trap in all of C++, [7:23] so again, before I do the big reveal, [7:26] I'm going to sort of increase the chrono ... [7:28] I'm going to put these in chronological order, [7:30] so this is sort of the order in which [7:32] you will probably encounter them in your career. [7:34] So hopefully, many of you have seen this already. [7:37] We'll end on something a little more hard. [7:39] I believe it to be maps operator square bracket operator. [7:44] Ooh, I heard whispers. [7:45] Yes, I've called it out. [7:47] So, I hope most of you know where I'm headed, [7:50] but if you don't, let's be clear. [7:52] Map has this property that ... [7:54] So in this case I've set hey to 12 in my map. [7:57] And then I try to print it out and I typo it. [8:00] Okay, I've typo-ed it, so I'm trying [8:02] to print out a key that doesn't exist in the map. [8:05] Most of us know what happens here. [8:07] It prints out zero, right? [8:10] And you're used to that, that seems normal to you. [8:14] Show this to a programmer who doesn't know C++ well [8:17] and ask them what they think this does. [8:19] The actual answer was no higher than third on their list. [8:23] This doesn't crash, this doesn't print out [8:25] an initialized memory, this doesn't throw an exception. [8:28] What this does is print out zero. [8:30] And it actually inserts something in the map. [8:32] So the size of the map at the end of this is two. [8:35] Okay? If you don't think that's weird, [8:38] you've been doing C++ for a really long time. [8:41] Yes. [8:43] We won't think twice about using this. [8:46] This is an occurrence count, right, [8:47] like a really dopey small one. [8:49] Like loop through the string, [8:51] and in my map, increment for every character I see. [8:55] Okay, you might write something like this. [8:57] The Java version of this is horrible, [8:59] because you have to initialize your variables, right? [9:01] And C++ just gets kind of, the integers get initialized, [9:04] they just get initialized somehow. [9:06] Don't let a beginner C++ programmer [9:08] stare at this for too long, [9:09] they start asking really complicated questions. [9:12] Like, wait a minute, you don't initialize any of your ints. [9:15] Nobody sets anything to zero. [9:17] How does that work? [9:18] Does that work with normal ints? [9:19] No, no no. [9:21] No no, it only works because of value, [9:22] oh I don't want to talk about value initialization. [9:25] Right, not with someone who's falling into this trap. [9:27] I want no part of like, [9:29] well see it's like a default construction of an int, [9:31] even though ints don't have default con ... [9:33] Okay, never mind, never mind. [9:34] Just, next slide, next slide. [9:35] Okay. [9:38] So, we actually protect our [9:40] sort of beginner programmers from this [9:42] because we drill into them const correctness. [9:44] Be const correct, it will save you from bugs, [9:46] this is one of those bugs that saves you from. [9:48] This code will not compile, okay? [9:51] You have a settings map that you're trying to read from, [9:54] with a square bracket operator, [9:55] but the settings map is const, so this won't compile, okay? [9:58] Because the settings square bracket operator [10:01] might need to insert, therefore it won't work, right? [10:05] No, yeah I ruined my own joke. [10:07] How does a newbie fix this code? [10:08] If you paid attention, you know. [10:10] How does a newbie fix this code? [10:12] - [Audience Member] Remove const. [10:13] Yes, remove const, thank you. [10:15] Yeah, you remove const. [10:17] Guess what, it works. [10:18] I know he told me to be const correct, but it works. [10:22] Yeah, so I was sitting with someone [10:24] who was struggling with something else, [10:25] they were relatively new to Facebook, [10:27] I saw code that was exactly like this, [10:28] and I said, whoa whoa whoa, why is this not const? [10:30] I already kinda knew why. [10:32] And they're like, well, it doesn't work when I put in const. [10:33] I'm like, yes, let's put it in, [10:36] let's look at the error message. [10:37] This is the error message. [10:39] Error: passing const std:: map dot dot dot, [10:41] as this argument discards qualifiers. [10:46] So as a C++ expert, you know what that means. [10:48] You can interpret that. [10:51] A newbie sees "error passing const," right? [10:56] The error literally says, [10:57] there's an error passing this const map, into this thing. [11:00] So I should remove it, it works, right? [11:06] Now of course only newbies fall for this mistake, right? [11:10] Of course. [11:11] Twice, twice as far as I know, major site incidents [11:15] have occurred in exactly this pattern. [11:18] Experienced, very experienced C++ teams [11:21] had a widget, in this case it was like a service handler, [11:23] in its constructor, it took a map of settings, [11:26] which is already gross, already gross. [11:29] And they assigned the data member settings [11:32] to the settings that gets passed in, [11:34] and then they print their settings. [11:36] Server initialized with timeout, whatever. [11:40] Well, what happens in a refactor [11:42] if you pass in a map that doesn't contain timeout, [11:45] well, we've just helpfully, in our print statement, [11:47] in our print statement, set the timeout to zero. [11:53] In most socket libraries, [11:54] a timeout of zero, infinite, yes? [11:57] So it's a really excellent way of making, [11:59] like, one service is dying, and now this service, [12:02] all of its threads are in infinite timeouts. [12:06] But of course, you know, everyone tests [12:08] all their timeout code paths really extensively, [12:11] so this doesn't happen, yeah I know. [12:15] So here's the bad news about this newbie trap. [12:17] It's actually the one we're the most powerless against. [12:20] I have nothing to help you, I have nothing. [12:22] Like people have legitimately suggested we just ban it. [12:25] Just ban the square bracket operator of map. [12:28] Const correctness helps to some degree, but not always. [12:30] So I got nothing, if you have a clever idea, [12:32] let me know, come find me, okay? [12:34] I'm interested. [12:35] (audience member shouts indistinctly) [12:36] What? [12:38] We'll call at, yeah but, so, [12:40] that's good if you know that, but they don't. [12:46] People complain about at for [12:47] different reasons, that's a different talk, okay. [12:49] So, good, at, at is one solution to this problem, [12:53] he mentioned at, because at will [12:55] throw if the key's not there. [12:56] Most languages have an equivalent [12:58] or rather an alternate mechanism which is extremely common, [13:01] which is this sort of get with default. [13:03] So you call it on, you can call the const map with the key, [13:05] and if the key's not there, instead of throwing, [13:08] return the default argument. [13:10] C++ doesn't have one of these, [13:11] turns out it's actually kind of tricky. [13:14] And this is the official actual implementation [13:17] in GHC Haskell, of this exact idea. [13:20] This is a real programming talk now. [13:23] We have Haskell in this talk. [13:25] We've put away the toys, this is Haskell now, right. [13:27] Welcome. [13:29] This is the actual implementation in Haskell, [13:30] it says, findWithDefault, default k m, [13:33] they're in reverse order in Haskell, okay. [13:35] Says look up the key in the map. [13:37] If you find it, or rather if you find nothing, [13:39] return the default, if you find an x, return the x. [13:42] That's what it looks like in the prelude, [13:43] or rather in the GHC standard library. [13:46] This is what the folly version [13:47] of this exact thing looks like. [13:49] So we have respectively this same function in folly. [13:52] It's not super fair to compare this, [13:54] because most of that noise is type information [13:56] and Haskell has type inference. [13:58] But this is a template that takes a map [14:00] and a key and the code down at the bottom is really [14:02] like what you'd expect, like find the key, [14:04] if it's not the ended narrator return it, [14:06] otherwise return the default. [14:08] That's what the sort of last two lines look like. [14:10] It's very straightforward. [14:11] There is something in this slide that C++ programmers hate. [14:16] They hate it, they hate it, they hate it. [14:18] (audience member shouts indistinctly) [14:20] Oh, I love this man, okay. [14:22] So I had a whole separate slide to build it there, [14:25] which is, I moved all the template noise [14:27] to make it more obvious, I said [14:29] let's get rid of all the template nonsense, [14:30] and I said let's just do the string version of this. [14:34] So this is the exact same slide [14:35] except with string substituted. [14:36] So I made it much more obvious now. [14:38] It returns a string by value, [14:40] that means there's a copy here, an unnecessary copy. [14:45] And when I was proposing this talk, [14:47] I was saying, hey, give me ideas for bugs [14:49] that have been recurring, so many people mentioned, [14:52] oh, having extraneous copies is a really common bug. [14:56] Oh, it is, isn't it? [14:59] Well how do you fix it if you hate extraneous copies? [15:01] Easy, right? [15:03] We know how to fix it, we return a reference to the string. [15:06] This works great if the key is in the map, [15:08] I don't have to copy anything out of the map, right? [15:11] I don't have to anymore, I'll just return [15:12] a reference to what was in the map, [15:14] and there's no more extra copies. [15:16] This is hopelessly broken. [15:18] This code is hopelessly broken. [15:19] Do not copy and paste this code. [15:23] It turns out, right, oh yeah, [15:25] this code is hopelessly broken. [15:27] Congratulations, we just crashed this site. [15:29] It turns out that people love default temporaries. [15:31] They love default temporaries, this is in fact [15:33] the way this function is almost always used. [15:36] On the right, you're creating a default string, [15:39] and if you return that by reference, [15:41] because the key's not in the map, [15:43] v is now a reference to a temporary [15:45] that has now been destroyed. [15:46] Yes? [15:49] This patch that I just showed you [15:52] has been submitted like four times, [15:54] and I've rejected it 3 of those times. [15:57] One time, I shouldn't say I, [15:59] someone rejected it three times. [16:00] One of those time it got in, and we're like, oh no no no. [16:02] I need to do negative caching of gifs and say, [16:05] if anyone submits this to just auto-reject it. [16:08] Okay, people want to make this change so badly. [16:13] But this is a broader class of problems, [16:14] this is a much bigger problem that's well known. [16:18] There's many creative ways to do this, [16:19] to pass temporary to an object and then [16:21] somehow return a reference to it, to the caller. [16:25] This is a really hard problem to fix, [16:27] I think, in the language. [16:28] There's some library tricks you can do often [16:30] to make it hard, harder to do this. [16:33] But at some point the language has to fix it, [16:34] and I don't know if it can or will. [16:38] But it can be mitigated, so again, [16:40] Address Sanitizer catches this, catches it cold. [16:43] The only problem is, you often need this extra flag. [16:47] So default Address Sanitizer, at least up to some point. [16:51] Didn't have this on by default. [16:52] I believe it's now turned on by default in Trunk, [16:55] I don't know which version that's actually going to ship in. [16:57] But when I turned it on, I had to add this flag. [16:59] And when I turned this flag on for ASAN, [17:01] we found dozens of bugs of this form, dozens. [17:04] It was bad. [17:08] All right, bug number four. [17:08] So this is intermission. [17:10] How are we doing on time? [17:11] So the intermission is interesting, [17:12] because I wanted to tell a story that works, [17:14] so this is not a recurring bug. [17:15] This is a bug that is not recurring. [17:18] This is a bug we killed. [17:20] It was horrible, it was a constant source of fighting, [17:24] and we actually fixed it. [17:26] It involves the worst keyword in all of C++. [17:30] Who knows it? [17:32] - [Audience Member] Auto five. [17:33] Auto, oh no, what else? [17:36] - [Audience Member] Go to. [17:37] Go to, oh, that's a good one, but no, not go to. [17:39] What else? [17:40] - [Audience Member] Volatile. [17:40] Volatile. [17:41] Volatile, oh. [17:44] Who knows what volatile does? [17:46] That's an honesty check. [17:47] Oh look at all these liar ... [17:48] Okay, I'm just kidding. [17:51] Yeah, volatile's a really scary one. [17:54] Lots of people think they know what it does. [17:56] At least historically, and use it really really wrong, [17:59] really really wrong. [18:02] We introduced what is actually in practice a terrible idea. [18:06] We throw a paragraph at you, if you write the word volatile. [18:09] Like our linter literally spews [18:11] a paragraph of gibberish at you. [18:13] It's kind of mean, but that's fine. [18:15] It's not actionable, which makes it [18:16] a terrible lint rule, but I don't care, [18:18] if you use volatile I'm going to lecture you. [18:21] Almost every use of volatile in our codebase [18:24] had to do with threading and trying to synchronize threads. [18:27] Now there are legitimate use cases of volatile [18:28] that don't involve threads. [18:30] They just don't exist in our codebase for the most part. [18:33] So you might live in a codebase [18:34] where you actually have memory map diode [18:36] and you need to do volatile. [18:38] But we don't. [18:39] So I had a really bad lint rule by most lint rule standards, [18:42] which said, if you write the word volatile [18:44] and it's not in a comment [18:45] I'm going to lecture you on your dev, automatically. [18:49] This really angered a lot of people, it was fun. [18:53] We'd get a post that was like, actually, [18:57] I'm using it correctly, I've one thread, [18:59] and there's a boolean, and he's writing to it, [19:01] and then the other thread's trying to read the boolean [19:03] and I don't want the compiler optimization, [19:05] so I make the bool volatile, [19:07] we've been doing this for 20 years. [19:08] You know, this kind of stuff. [19:12] We had so many of these arguments, [19:14] but the thing that's interesting is that we won. [19:15] We won the arguments. [19:17] Like people have been at Facebook for a few years, [19:18] they don't remember volatile arguments. [19:21] The Great Volatile Wars of 2010, they're gone, 2011. [19:26] People stopped using volatile for threads, [19:29] and I think if you pause for a second, [19:30] it's kind of interesting, like, why? [19:32] How did that actually happen? [19:34] It turns out people wanted atomic flags [19:36] or mutexes or higher, yet higher order abstractions. [19:40] And once we started giving it to them, [19:42] somehow volatile just vanished, [19:44] and we sort of trained the old guard who had learned. [19:46] But the new people who wandered [19:47] into the language must be just [19:48] finding what they want, the right thing. [19:52] The use of volatile synchronization mechanism [19:54] seems to have mostly vanished. [19:57] And it's, I'm not exactly sure why, [19:59] but I think it's an interesting case study, [20:00] and sort of, people really want to do x, [20:02] and they'll find a way to do x. [20:04] And maybe if you just give them a solution, [20:05] they'll use it, turns out. [20:08] All right, bug number five. [20:11] This is the worst question in all of C++. [20:14] This is the one that makes my blood run cold. [20:17] The way this usually works is [20:18] someone posts in our internal group. [20:21] They tell a story, the story is always the same. [20:23] It goes like this. [20:24] We've been having an intermittent crash for a month. [20:27] It's really hard to reproduce, [20:28] I can't reproduce it. [20:30] What we've ended up having to do is bisect it in production. [20:33] We build a binary, we run it for six hours, [20:36] if it crashes then that's bad and if it doesn't that's good. [20:39] We've been bisecting for like 3 weeks. [20:42] We finally found the diff and [20:45] we don't know why this diff is broken. [20:48] We've been staring at it and ripping it apart and we don't, [20:50] there's something here we clearly don't understand. [20:53] But we finally came up with a hypothesis. [20:55] And then they ask you the question. [20:57] It's only three words. [20:59] Is ... [21:01] Shared_ptr ... [21:04] Thread-safe? [21:07] (laughs) Is shared_ptr thread-safe? [21:10] What's the answer? [21:11] - [Audience Member] No. [21:12] No, I like that. [21:13] Any other answers? [21:14] (audience member shouting indistinctly) [21:16] I'm sorry, I couldn't hear, it's not this ... [21:18] You hear answers like this, like yes, no, it depends. [21:23] People love this answer, which is totally useless but true. [21:27] "It's exactly as thread-safe as a normal ptr." [21:31] Technically true, but useless. [21:37] If you go to Google and you type in [21:39] is shared_ptr, the top result. [21:43] Is shared_ptr thread-safe? [21:45] By the way, the third result, is shared_ptr slow. [21:49] No, no shared_ptr is not slow. [21:51] If it's in your profile, fix your code. [21:55] Is shared_ptr null is fourth. [21:58] Yes. [21:59] Yes, shared_ptr is null. [22:00] Put a check in. [22:01] Okay. [22:02] But we're here about thread safety, all right. [22:03] Is shared_ptr thread-safe? [22:08] So I've had a lot of time to come up with an answer. [22:11] I've had a lot of practice. [22:13] I believe the answer to, [22:14] is shared_ptr thread-safe, to be no. [22:18] That is technically, I believe, to be a lie. [22:23] But I'm answering a slightly different [22:25] question than the one you're asking. [22:27] You asked, is shared_ptr thread-safe, [22:30] but what you meant to ask was, [22:31] is my code correct? [22:33] And the answer, if you have to ask, is no. [22:36] No, no. [22:38] So here's my teaching sense, [22:39] so you might have no idea what we're talking about. [22:41] You might be in audience one right now, [22:42] going what are these people talking about, [22:43] what do you mean thread shared_ptrs and thread-safe, [22:44] let me explain it to you. [22:46] A shared_ptr, this is a mental model, [22:48] this is not an implementation, it's a mental model. [22:50] It's three things. [22:52] There's the actual shared_ptr struct, [22:54] which is a pair of pointers, [22:55] that's what it is under the hood. [22:57] Doesn't have to be, but in our [22:58] mental model that's what it is. [23:00] It points to two things. [23:02] One pointer points to the shared T, [23:04] one points toward the shared reference count. [23:08] This is where the C++ people get really uncomfortable. [23:10] They're like, oh, it's not a reference count. [23:12] There's a lot of other stuff in there. [23:13] It's technically called a control block, [23:15] and it contains, you know, meet pointer, [23:17] dah dah dah dah dah. [23:18] But for mental model purposes, [23:20] it's a reference count, okay? [23:22] And the answer, or rather, yeah, when you copy it, [23:25] all you do is you copy the struct with the two pointers, [23:27] and they point to the same two shared things, [23:29] the shared reference count and the shared T, [23:31] and then we increment the reference count, all right? [23:33] That's how shared_ptr works under the hood, [23:34] that's the mental model. [23:36] The thread-safety kind of approximately looks like this. [23:40] Anything that touches T is unsynchronized. [23:43] The shared_ptr will not help you synchronize the T. [23:46] That's on you. [23:47] If you want synchronization at your T, do it yourself. [23:51] The shared_ptr will, however, manage the control block, [23:54] and in particular the reference count. [23:55] So if multiple threads destroy shared_ptrs, [23:58] they will both atomically decrement a reference count. [24:00] That will happen. [24:02] But the one that trips people up [24:03] tends to be this thing on the left. [24:05] For some reason, people want to read and write [24:08] to a shared_ptr unsynchronized. [24:10] It doesn't work. [24:11] For the exact same reason a pair [24:12] of pointers wouldn't work from two threads. [24:16] This is a thing that for some of us seems super obvious, [24:20] so I was testing this with people at Facebook. [24:23] Like, do you know this? [24:25] Is this obvious, does everyone know this already? [24:27] And one person literally went, that's the crash. [24:31] That's the crash. [24:32] We have a map, and the shared_ptrs, that's the crash. [24:35] And I was like, okay, I guess that's going in the talk. [24:41] I've changed the definition of the colors now. [24:43] I want to talk about our intuition now. [24:44] So now I'm moving towards audience two. [24:47] You hire lots of programmers [24:49] and you're training them over time [24:51] and you should know this about their intuition. [24:53] They get the right hand side correct. [24:56] They naturally assume that T is unsynchronized [24:58] when playing with shared_ptrs, they get it, just intuitive. [25:02] They naturally assume that the control block, [25:04] the reference counting, happens for them automatically. [25:06] The thing they seem to get wrong is the one on the left. [25:10] People really want to read and [25:11] write to shared_ptrs unsynchronized. [25:17] The pattern that comes up over and over and over at Facebook [25:21] and I think that this is an artifact [25:22] of the fact that we write a lot of services, [25:24] is this, it's a buggy form of RCU, [25:26] and it's actually a degenerate but very common form of RCU. [25:30] RCU, by the way, is Read-Copy-Update, [25:32] has a Wikipedia page that was to talk about it. [25:34] I'm not going to talk about it too much here. [25:36] But in our most common use case it's actually very simple. [25:39] You have a service, it handles requests, [25:41] you have one thread who's responsible as the updater. [25:44] So one thread wakes up every 20 minutes or whatever, [25:46] goes and reads some database, [25:48] populates some in-memory efficient version [25:50] of whatever's in the database, [25:52] and then stores that into a shared_ptr [25:54] as the one true most recent state in the universe. [25:58] The worker threads go into that [26:00] one true shared state at the start of their response, [26:03] they pull one out, grab a reference count to it, [26:05] and say, okay, I'm going to use this [26:06] state of the universe for this request. [26:08] And then on the next request, [26:09] I'll grab a new state so you won't update me [26:12] between whilst I'm in a request, [26:14] but I'll pick up the update after I'm done. [26:16] And this is really, this has [26:17] a lot of lovely properties in a service, right, [26:19] because updates are really cheap. [26:21] You're not really blocking anything to do your update. [26:24] It has the downside, of course, [26:25] that you might have multiple states in flight, [26:27] like whilst you're updating one state, [26:28] someone might be using the old one, [26:30] and you've gotta be able to tolerate [26:32] several states in memory at once, [26:34] to have this work effectively. [26:37] But, and the implementation here is actually really easy, [26:39] all you gotta do is at the heart of that handoff, [26:41] have a shared_ptr and a mutex. [26:43] And have an atomic get and an atomic set and you're done. [26:46] Right, it's not even that. [26:47] It's literally the easiest thing to write. [26:50] But what's interesting is our intuition fails us. [26:51] People keep writing the wrong thing, [26:53] and then they have a really bad month. [26:56] Finding that bug takes a month, [26:58] if they don't see it or know it. [27:03] So mitigation, yeah. [27:03] So thread sanitizer finds this, if you run thread sanitizer. [27:07] Address Sanitizer often finds it as well, [27:09] but it's not super helpful. [27:10] Because what happens up happening [27:11] is you end up corrupting the shared_ptrs [27:14] and you destroy the wrong thing [27:15] and it looks like memory corruption, [27:16] and it's not always clear. [27:18] Address Sanitizer will tell you something's wrong, [27:21] but it won't necessarily aim you right at it. [27:26] Now we get to the library part of the talk. [27:28] There has been a thing called atomic_shared_ptrs in C++11. [27:31] On paper, this solves the problem. [27:33] Of course, most implementations haven't had one. [27:36] And then there was a concurrency test [27:37] that added atomic_shared_ptr like this. [27:39] This here, atomic_shared_ptr like this, and then like this. [27:42] And then we went back to atomic_shared_ptr like this, [27:44] like I heard something about a vote in Toronto. [27:46] I asked someone in SG1. [27:48] so I tend to hang out in the [27:49] evolution working group of the committee. [27:50] I don't really hang out in the SG1 [27:51] which is where they talk about this. [27:53] I was like, can you give me the history of this, [27:54] because I'm going to give a talk, [27:55] and I'd like to at least tell people [27:56] what the deal is about atomic_shared_ptr. [27:58] They're like, eyes glazed over, [27:59] and apparently this is a big long interesting, [28:01] I don't know about interesting. [28:03] This is apparently a big long topic of back and forth [28:06] of what's going to happen with atomic_shared_ptr. [28:08] I don't actually know its current state, [28:09] I suspect we'll get something like it at some point. [28:12] It does solve my problem, [28:13] but to be fair I don't need a [28:15] highly performant atomic_shared_ptr to solve my problem, [28:18] I can get away with a mutex and a lock. [28:21] And in fact, there is also another proposal [28:22] if this is interesting to you, [28:24] about a cell type of an abstraction, [28:26] which is effectively exactly what I want. [28:27] That's also before the committee. [28:29] Oh, bonus bug about shared_ptrs. [28:31] We have this one all the time too. [28:33] I don't know why people write this. [28:34] I can't explain it. [28:36] Some function returns a shared_ptr, [28:38] they just dereference it, immediately, [28:40] and they grab a reference to whatever's at the other end. [28:43] And now you call ref.boom(). [28:45] So the problem here of course is everything [28:48] that happens after that, the shared_ptr's gone. [28:50] It's been destroyed, because you dereferenced it. [28:52] And you basically, you don't have [28:55] the protection of a reference count anymore, right? [28:58] So all your code is now running [28:59] without a reference count protecting you. [29:01] So this one's come up a number of times, [29:03] but again Address Sanitizer does catch this one. [29:05] Very easily. [29:08] All right. [29:09] Let's see. [29:10] All right, last one, this one's my favorite. [29:13] Bug number six. [29:14] This is code review, this is going to be interactive. [29:16] At least a little bit. [29:18] I'm going to show you a series of slides [29:19] with very little code on it. [29:21] Very little code. [29:22] And I have two questions for you. [29:24] Question one, does it compile? [29:26] Everyone's favorite C++ game. [29:28] Will this code compile? [29:31] Question number two, if it doesn't compile, [29:33] what error message would you expect or even like, [29:38] if you're being ambitious? [29:40] So, does it compile, what kind of [29:41] error message do you expect? [29:43] Here is the code. [29:46] Include a string. [29:48] Void f. [29:49] Standard string, parentheses foo, [29:51] close parentheses, semicolon. [29:55] Raise your hand if you think this code compiles. [30:01] So, for the purpose of the audience, maybe 10%. [30:04] Maybe 10%. [30:06] But I pointed that because [30:07] that's Richard Smith, right there. [30:08] You're, what, the editor of the C++ standard, right? [30:11] Yes. [30:12] The editor of the C++ standard says this code compiles. [30:16] Who's changed their mind? [30:17] (audience laughing) [30:20] Richard, unsurprisingly, and surprisingly, is correct. [30:24] This code compiles. [30:27] Almost everybody in here got it wrong. [30:29] This code compiles, what? [30:33] What? [30:34] Okay, take a minute, pause the video, if you're on video, [30:38] think really hard about how [30:39] this code can possibly compile. [30:41] How this code can possibly compile. [30:44] And then think a little bit more, like more existentially, [30:46] like what are we doing, why are we here? [30:49] I feel like I'm wasting my life a little. [30:50] Okay, this code absolutely compiles, [30:52] and the answer, and I'm not an expert in the standard, [30:55] but my understanding is, the way the standard works, [30:59] at least in this part of the standard, [31:00] if something looks like a declaration, [31:02] it is a declaration, no matter what else it might be. [31:06] And that looks like a declaration. [31:08] So, this, and this, are basically the same line of code. [31:12] Those parentheses are optional. [31:14] So what you're seeing, on the top, [31:16] is a declaration of a string called foo. [31:20] Okay? [31:21] Now, that seems like trivia. [31:25] That seems like that seems weird, like, okay, how. [31:28] Oh yeah here's me proving it, by the way. [31:30] So you put it into Godbolt, [31:31] and you can't really see this, [31:32] but what you'll see in Godbolt with no optimizations [31:34] is the default construction of an empty string, [31:37] and then the destruction of the string. [31:39] So you'll see the creation and [31:39] destruction of an empty string, foo. [31:42] So, okay, but the question becomes like, so what? [31:45] How is this possibly one of the most [31:47] common and horrifying bugs in all of C+? [31:49] And I posit to you that it is, just not in this form. [31:52] It's in a slightly different form. [31:54] This is often what it looks like. [31:56] You have an object that has an update method, [31:59] and this object is meant to be used from multiple threads. [32:04] So it needs to be internally synchronized. [32:06] And so you of course have a data member that's a mutex, [32:09] and on any mutating method you have [32:11] to lock your mutex, right? [32:13] That's the way it works. [32:14] So you lock your mutex, and then you do the mutation. [32:20] Right? [32:21] Everyone sees the bug, right? [32:23] Do you see it yet, who sees it? [32:26] Only half of you, okay. [32:28] Yeah, so I didn't name my lock here, right? [32:32] I am not, that unique lock is [32:34] intending to be like a lock guard, right? [32:36] I'm intending to, on construction, [32:37] lock my mutex, and on destruction unlock my mutex. [32:42] That's what I'm hoping this code does. [32:43] That is not what this code does. [32:45] No, what this code does is it [32:47] declares the existence of a unique lock, named my mutex, [32:53] that shadows the mutex data member [32:56] that you were trying to lock. [32:58] And to be clear, nothing gets locked. [32:59] There is no lock that occurs here. [33:02] It looks like it's locking, but you want it to lock. [33:04] But it's definitely not locking, what you want to lock. [33:06] And the fix is super easy. [33:07] You've seen it a million times. [33:09] You have to name the lock guard. [33:11] You have to give it a name. [33:12] Now, this is a unique log named g, [33:15] whose construction locks the mutex [33:17] and whose destruction unlocks the mutex. [33:20] So your code is now protected by a lock guard, right? [33:28] Do a code search for this. [33:30] This is the pattern, do it. [33:32] Unique_lock, if you have a code [33:34] that's using relatively recent C++, [33:37] do a code search for unique_lock. [33:39] GitHub search won't let me do it, [33:41] so I couldn't do it on GitHub, [33:43] but I figured since I'm giving this talk [33:45] and I'm telling all of you to search your code, [33:46] I should search mine. [33:48] Do you think I found a bug? [33:51] Yes. [33:53] This is what the actual code looks like, [33:55] std::unique_lock (mtx). [33:58] That is not doing what it thinks it's doing. [34:00] That is not a lock guard. [34:02] That is a shadowing unique lock [34:05] that is not locking the mutex it tried to lock. [34:08] I didn't just find one bug, [34:10] I found two bugs in our codebase. [34:13] Here's the sad thing about this. [34:15] I only found two bugs, which is okay, [34:17] but we actually have a lint rule to protect against this. [34:21] So this is two bugs that got into our code [34:23] despite the linter telling them not to do this. [34:27] Imagine how many we would have [34:28] if we didn't have a linter telling people not to do this. [34:34] All right, now we're going to do [34:35] the worst C++ quiz of all, all right? [34:39] I've taught you everything you need to know, [34:40] there's no more tricks, [34:41] I've taught you everything you need to know [34:42] to understand what happens next. [34:44] So does this compile? [34:45] So remember, this compiles, this compiles. [34:48] Shockingly, perhaps, this compiles. [34:50] Does this compile? [34:57] Raise your hand, does this compile, if it does compile, yes. [35:00] Only half of you. [35:02] So this compiles as well. [35:04] This compiles as well. [35:06] The first one, as we mentioned, [35:07] is the declaration of a string named foo. [35:10] The second one is a constructor. [35:13] It constructs a temporary string from a char star. [35:18] All right, let's make it worse. [35:19] Does this compile? [35:21] I wrote it twice, it's copy and paste. [35:24] Raise your hand if you think this compiles. [35:27] No, no this doesn't compile. [35:28] (laughing) [35:31] No, this is a redefinition error. [35:33] This is a redefinition error, [35:34] because if it looks like a declaration, [35:35] it is a declaration, [35:36] so this is the same as starting standard string foo, [35:37] standard string foo, [35:38] and we'll say you can declare the variable [35:40] with the same name and the same scope. [35:42] So this is a redefinition error. [35:44] All right. [35:45] Does this compile? [35:46] Look closely, what? [35:47] We have a question? [35:48] (audience member speaks indistinctly) [35:50] It could be interpreted that way. [35:52] It could be interpreted the [35:53] second one is a copy of the first. [35:56] But the rule is if it's ambiguous, [35:59] and it could be a declaration, [36:00] then it is a declaration. [36:02] So this is a redefinition error. [36:04] To be clear, I'm slightly out of my league here, [36:06] because now you get into super-parsing of the standard, [36:09] but as far as I tested nobody will compile this, [36:12] and I believe that's why. [36:14] Because the rule says if it's ambiguous with a declaration, [36:16] then it is a declaration, period. [36:17] And that's why you get a redefinition error. [36:21] Right, does this compile? [36:25] All right, yeah, most of you've got it. [36:27] Yes, this does compile. [36:28] The first one is a declaration of a string named foo, [36:33] and the second one is a temporary [36:35] copy constructor of the string. [36:37] It's calling the string's copy constructor [36:39] with the first foo, which is doing it okay. [36:41] All right. [36:43] And then finally, just for symmetry purposes, [36:45] you grab the squiggly braces and you pull them out. [36:48] Does this compile? [36:52] Yeah, all right, most of you, yes. [36:53] Most of you say yes, this compiles. [36:55] So the reason this compiles, [36:56] this is again a second declaration of foo, [36:59] but it's now in an innerscope. [37:01] So the squigglies are a scope, [37:02] and it's now shadowing the outer foo. [37:04] You have two declarations of variables with the same name [37:06] but one of them's in an innerscope shadowing the outer one. [37:08] Right, that's what's happening here. [37:09] So this works. [37:14] So, some super important notes. [37:16] There's actually a subtlety [37:16] here that's critical to understand. [37:21] There are two things that make unique_lock conspire [37:24] to have this really pathological [37:26] bug-prone behavior, as far as I'm concerned. [37:29] The first is its RAII type. [37:32] If you have the exact same situation with [37:34] a non-RAII type, you tend to notice very quickly, [37:37] because you didn't name it or anything, [37:38] so you'll try to use it or something [37:40] and you'll be like whoa, okay, I forgot the name. [37:42] But RAII types, their mere declaration [37:45] is often all the functionality you ... [37:46] That is the functionality. [37:48] I declare it, right, and that means I've got one. [37:51] So you're much less likely to name it, [37:53] because you don't actually need a name, [37:54] you never need to reference it. [37:57] The second part that's actually really important [37:59] is this only works because [38:00] standard_unique_lock has a default constructor. [38:03] Because I default construct mutex, in this case. [38:05] I have to use the default constructor. [38:07] So unique_lock is affected, but lock_guard is not. [38:11] So if your standard lock_guard will not compile, [38:14] in the case of, if you were to swap that out. [38:18] Now, if you're using standard types that's great, [38:21] because standard unique_lock has [38:22] this weird property you can lint on it fairly easily. [38:24] If it's like our codebase, [38:26] you have lots of reimplementations of [38:28] lock_guards from the years, right? [38:30] Like, dozens of them. [38:32] And very few people fully appreciated this problem. [38:35] So we found, I don't know, [38:38] tens, maybe 100 cases of our other lock_guards [38:42] that did have default constructors [38:44] being bugged in this fashion. [38:46] So if you have a lock_guard, [38:47] run, don't walk, to see [38:49] if it has a default constructor. [38:52] By the way, unique_lock does. [38:54] C++17 has a thing called scope-lock, [38:57] and it's entirely unclear to me [38:59] if it suffers from this problem or not. [39:01] This is complicated. [39:03] As of yesterday, when I put it into Godbolt, [39:05] Clang will not compile it and GCC will. [39:09] I'm not sure. [39:10] So there's, scope-lock may or [39:12] may not have this problem in C++17. [39:16] So anyway, the TL;DR on all this [39:18] is if you have an RAII type [39:19] and it has a default constructor, [39:21] you're in a very dangerous situation, potentially, [39:24] where people will try to wrap something [39:26] and forget to name the object [39:29] instead of what they're wrapping [39:30] and they'll end up shadow ... [39:31] Declaring a shadowing thing. [39:34] So that's my other takeaway for you. [39:38] All right, so mitigation. [39:39] So, Wshadow is a, at least in GCC and Clang, [39:43] is a warning that's been around a while. [39:45] It absolutely detects this, Wshadow, [39:47] so shadow tells you about shadowing warnings. [39:49] That's when you have a variable in an innerscope [39:51] that shadow a variable on an outerscope, it will warn you. [39:54] If you put this particular pattern in [39:56] and you turn on Wshadow, it will absolutely find this bug. [39:59] The problem, of course, if you've ever used Wshadow, [40:01] is it's extremely pedantic. [40:04] It almost always gets turned off. [40:07] It's full of noise and things [40:09] that you probably don't want to fix, [40:10] and makes the code maybe worse. [40:13] There have been these hybrid flag suites, [40:15] so there's shadow-compatible and shadow-compatible-local. [40:18] These are meant to be much [40:20] higher signal versions of Wshadow, [40:22] so shadow-compatible only will tell you [40:24] about shadowing warnings if [40:25] the two types are compatible with one another. [40:27] And shadow-compatible-local is only [40:29] if they're local in the same function, [40:30] and they're compatible with one another. [40:32] I strongly encourage you to [40:33] turn on at least Wshadow-compatible-local. [40:36] You will find all kinds of, if not broken code, smelly code. [40:40] Like there's an outer loop using I [40:42] and a bunch of inner loops using I, [40:43] and it's like let's just not do that. [40:45] Even if it's not bugged, you don't want it to be, right? [40:48] So shadow-compatible-local is good, [40:49] you should turn it on, but it does not catch this. [40:51] Only Wshadow catches this bug. [40:54] So, I'm sorry. [40:56] The story there isn't great. [40:58] But, you know, this means this is easily detectable, right? [41:01] Our extendable Clang based linter has a check for this. [41:04] It's actually fairly easy to detect. [41:07] But it's actually not widely detected. [41:08] If you don't have a tool that finds this pattern, [41:11] which is to say, unnamed lock guards, [41:14] this sort of immediately wink out of existence. [41:18] I suspect you have a lot of these, [41:19] especially if you have default constructors [41:21] on your lock_guards, or you use unique lock in your code. [41:26] Again, I'm not exactly sure what the best ... [41:28] I have some extra stuff here that [41:30] I didn't want to get into in too detailed. [41:32] There's a related class of problems [41:34] which isn't this declaration issue, [41:36] but it's more like you create something [41:38] that exists for one line of code [41:39] only when you expected it to live forever. [41:41] That's a place where a nodiscard attribute [41:43] for constructors might be helpful eventually. [41:47] So it's like two flavors of this, [41:48] but the more common one is the declaration one. [41:51] So again, we have a Clang based linter, [41:53] so something like clang-tidy, for example, [41:55] could very easily find this. [41:58] All right, so conclusions. [42:03] C++ is hard. [42:06] That's not fair, it's kind of a value judgment, [42:08] but we should just admit it, I think. [42:10] C++ is hard. [42:11] Like a lot of this stuff here, [42:13] we've had some of the best C++ people [42:15] spend a week finding a bug, [42:17] and at the end track it down to, [42:18] like, their lock_guard is missing a g. [42:21] And like, that stinks. [42:23] And the consequences of that are really, [42:26] it's a painful thing to be bugged. [42:27] It's an incredibly painful day to find that bug. [42:31] And it's a one character change. [42:33] The patch is one character. [42:36] Tools are our best teaching weapons. [42:37] So, lectures are great. [42:40] I would love to put every Facebook programmer [42:43] through a lecture that just taught them [42:45] about all the bugs that they should never write. [42:47] Like, does it actually work all that well? [42:50] The sooner after you write the bug I tell you, the better. [42:54] Everyone would pretty much agree with that. [42:56] If your IDE tells you [42:57] you just wrote a bug, that's fantastic. [42:59] The compiler tells you you just [43:00] wrote a bug, that's pretty fantastic. [43:03] If your dev review tool runs [43:06] some expensive asynchronous thing [43:08] and comments on your change set and says, [43:10] oh that's definitely a bug, [43:11] that's pretty good, that's pretty good. [43:13] If you get an e-mail two days after [43:15] you committed it because some monstrosity, [43:16] like some supercomputer did some [43:19] interprocedural static analysis and e-mails you a bug, [43:21] that's still not terrible. [43:23] Not great, but not terrible. [43:24] If I have a talk like this and two years [43:27] after you spent a week debugging it, I explain it to you, [43:30] that's borderline useless. [43:34] So really, for a lot of this, [43:37] for my money, I want better tools. [43:41] I'll talk in a minute about this, [43:42] but the sort of Clang extendable linters [43:44] have been really powerful internally [43:46] in terms of, for a particular data type, [43:49] having specific rules we can check, [43:51] checking certain kinds of semantics. [43:52] I don't want lock_guards that shadow mutexes, for example. [43:55] That's a really good rule. [44:01] Clicker. [44:02] Yeah I just said this. [44:02] So yeah, extendable linters. [44:03] So Clang-tidy is an example of this. [44:04] We have one internally called how to even. [44:07] Because you can't even, they're clever, they're clever. [44:13] An Address Sanitizer. [44:13] Yeah, like I didn't really intend for Address Sanitizer [44:16] to be the solution to all of my problems in this talk, [44:18] it just worked out that way. [44:20] I've already ... [44:21] One cannot speak highly enough of Address Sanitizer, [44:24] and the amount of bugs it finds in a codebase. [44:26] If you live in a codebase that compiles with Clang or GCC, [44:29] and you've never turned Address Sanitizer on, leave. [44:32] Go do it right now. [44:33] Go to one of the desks. [44:35] I hope all of them are turning [44:36] on Address Sanitizer right now. [44:38] It's really, it's life altering. [44:41] You'll find a dozen bugs in your codebase, [44:43] running your test suite, with Address Sanitizer [44:46] turned on for the first time. [44:48] You know, don't tell anyone until you've asked for a raise, [44:51] but then teach your team about Address Sanitizer. [44:54] You're welcome. [44:55] Okay, questions? [44:57] I'm done. [44:58] (audience applauding) [45:08] If there's no questions, I'm going home. [45:11] All right. [45:12] No questions. [45:14] Oh, no no no. [45:19] - [Audience Member] What bugs didn't make the cut? [45:23] What bugs didn't make the cut. [45:24] Everyone kept trying to say unnecessary copies, [45:28] and I was like, I have a huge bug that I keep having [45:31] because you all hate unnecessary copies so bad. [45:33] So that one didn't make the cut. [45:36] A lot of people want to do small things, [45:37] like pass by reference instead of pass by value. [45:41] Just going to pick all the ones Nicholas [45:42] just submitted, a few minutes ago. [45:44] The other one that's really common, [45:45] so I actually think one of the most important bugs in C++ [45:47] that we haven't talked about that's actually really hard [45:49] is asynchronous programming and lifetimes. [45:51] So we have Folly Futures, [45:52] and we have fibers, and we have a lot of lifetime problems [45:55] related to asynchronous programming. [45:58] And other languages that have solved this, [46:00] like have asynchronous programming [46:01] frameworks have garbage collectors, [46:02] which help them with all the lifetime issues. [46:04] So we have a lot of problems related to that. [46:05] That was a very popular nomination, [46:07] but we didn't ... [46:08] It doesn't fit on the slide. [46:11] But I do think that one's actually really critical. [46:12] Asynchronous programming and core teams will help with this, [46:14] and lifetimes become a really tricky balance. [46:18] So that's a short answer. [46:19] Okay, who's next? [46:21] - [Audience Member] Early on, you had a slide about a diff [46:23] that people kept wanting to submit on your get default. [46:26] Yeah. [46:27] - [Audience Member] Why wouldn't you put a comment there [46:28] saying, you might expect this to be this, but don't do it. [46:31] Yeah, I mean, so I have a skipped slide, I won't go to it. [46:35] No, what we ended up doing was [46:36] making a overload to that function that takes [46:38] the r value default and delete, equals delete. [46:41] That solves most of the problem, [46:42] and then put a big comment there. [46:43] That's what we ended up doing. [46:45] The actual more complicated lie here [46:47] is that we've, or rather the more complicated truth [46:49] from the simple lie that I told. [46:51] We have more than one of these. [46:53] So the folly one we fixed, [46:54] but there was a different one in [46:56] our STL utils that was some ancient code, [46:59] and that one had a bug in it that [47:01] got committed without us noticing. [47:03] So, that's what happened. [47:07] - [Audience Member] Will return an optional [47:09] to a value save my map index operator? [47:13] Will returning an optional from the map index operator? [47:17] - [Audience Member] Return optional to reference to a value. [47:21] Will it save this index operator? [47:24] Yes, do you ... [47:26] Yeah, so square bracket operator [47:28] on map returning an optional, on a const map. [47:31] So I don't hang out in library, [47:33] my suspicion is that that particular thing is ... [47:36] Everyone would like something like that, eventually. [47:37] It's complicated. [47:39] For reasons that, oh, optional's another good one. [47:41] Just corner a library person and ask them [47:43] about optional and all the ways that it gets hard. [47:45] But yes, I agree, that is, [47:46] that's what Haskell does, right, it returns a maybe type. [47:49] So that's what you want, you want [47:50] an optional as your square bracket operator. [47:52] So I agree, that's the right [47:53] direction to head in eventually. [47:55] - [Audience Member] Thank you. [47:56] - [Audience Member] But standard optional [47:57] doesn't support references. [48:00] Someone who would know better than I, yeah. [48:02] - [Audience Member] So my actually question is, [48:03] will C++17 rules regarding mandatory copy elision [48:08] simplify or complicate the concerns about too many copies? [48:14] I don't know, what do you think? [48:16] - [Audience Member] That's why I asked you. [48:17] (audience laughing) [48:21] I think we're rightly worried about copies, [48:24] so mandatory copy elision is good. [48:27] I just, you know, not all APIs, [48:30] especially ones that take references are, [48:32] can return references, right? [48:35] That's part of ... [48:38] I don't know. [48:39] I don't think so. [48:40] I think copy elision is good. [48:41] On average, it'll be good. [48:42] I think the broader problem of, [48:44] it's really easy to smuggle a reference [48:45] out of a function, is its own problem. [48:47] I don't know how or if we can fix it. [48:50] Do you think we can fix it? [48:53] - [Audience Member] I'm wondering if people will be [48:56] less scared about returning copies [48:58] so we just don't run into the problem, [49:00] or if we start talking too much about is this copy [49:04] getting into a more complicated spatial language? [49:07] Yeah, well I mean, so the example of get default, [49:09] you have no choice, because you have more than one source. [49:13] You can't elide the copy. [49:14] If it's in the map, you have to copy it out. [49:17] So in that case, it won't help, [49:18] but on average you're probably right. [49:19] It'll make it much less likely for [49:21] people to be paranoid about copies. [49:24] Dmitri. [49:25] - [Dmitri] These linters looks really useful. [49:28] Do have plans to open source them? [49:30] What looks ... [49:31] - [Dmitri] Linters, like Clanger based. [49:34] So, we built ... [49:38] This is a long story too, we built how to even, [49:40] which is effectively clang-tidying right before [49:43] we had a clang-tidy that we could use. [49:45] So we sort of built our own version of this thing, [49:47] and there's been great long term discussion of, like, [49:49] if we open source it, are we just bifurcating the space. [49:52] Should we convert all these checks [49:54] to clang-tidy checks and go that way? [49:56] So this is an ongoing discussion. [49:57] Yeah, I want to open source stuff, [49:59] like we should open source all these tools, and we often do. [50:04] We got to figure that out, that's the real issue. [50:06] It really is just a simpler version [50:08] of clang-tidy, that's what it is. [50:10] So I'm not sure how valuable [50:11] it would be, in its current state. [50:17] - [Audience Member] This is more coming [50:18] for the audience actually, [50:20] so when Facebook copies 100 bytes too many, [50:25] it costs them one million dollars. [50:28] Me and you, it costs us zero. [50:31] You have so much bigger problems than [50:33] just copying a string one too many times. [50:37] I just had to say it out loud, sorry. [50:41] It's true. [50:42] But the problem is if you're a library author, [50:45] you have users who care. [50:48] Occasionally. [50:50] Often times, even the ones who do care, care too much. [50:54] It's like, no, I don't see it in your profile, go away. [50:57] But fair point. [50:58] Tony? [50:59] - [Tony] Have you brought up the scope-lock constructor? [51:03] So I've discussed this with people in SG1, [51:06] and they said to do a DR, and I said I'm not actually very-- [51:09] - [Tony] Library evolution, library, talk to those guys. [51:11] Oh okay, yeah. [51:14] Yes, I've had several conversations with these people, [51:16] but I'm not actually sure if it's broken. [51:17] Or, I don't know if broken is the right word, [51:19] but I'm not sure if it suffers from this behavior. [51:21] I have to spend some more time. [51:22] I originally thought it did, [51:23] and then I was going to go ahead and do it, [51:25] but then on second glance it interacts [51:27] with some other weird stuff. [51:28] Because it's a very added template, so it actually, [51:29] and then there's argument deduction now, [51:31] so it's complicated. [51:32] So I'm not actually sure if it [51:34] suffers from this behavior or not. [51:36] So if someone wants to-- [51:37] - [Audience Member] Well, there's people here [51:38] I think could probably answer that for you. [51:39] All right, I'll ask them. [51:41] Who should I ask? [51:42] - [Audience Member] Well you got, [51:43] you got a few extra tools ... [51:45] (audience member talks indistinctly) [51:49] I know, poor Richard, he didn't know [51:51] what talk he was coming to. [51:52] Okay, any other questions? [51:54] We all set? [51:55] All right, thank you, everyone. [51:56] (audience applauding)