# Exercise 02 — The totals that are always a bit low

**Failure mode:** data race on an unsynchronised read-modify-write
**Language:** Swift 6 · **Runtime:** about 2 minutes with the sanitizer builds · **Difficulty:** foundational

---

## The prompt

> A download manager tallies bytes and chunks from several transfer threads. QA reports
> the totals are "a bit low, but only on fast connections". The engineer who wrote it says
> they have run it twenty times and it looks fine.
>
> Prove the defect exists with something better than a total that looks wrong. Fix it.
> Then explain why their twenty runs proved nothing, and why a build flag can change the
> answer.

Answer before you run anything: **is `counter += 1` one operation?**

---

## What you are given

| Path | What it is |
| --- | --- |
| `broken/stats.swift` | The starting point. Loses updates. Do not edit it — copy it. |
| `fixed/stats.swift` | One correct repair. |
| `solution.patch` | The diff between the two, applies with `patch -p1`. |
| `check.sh` | Proves the broken symptom and the fixed result. |

`DownloadStats` is marked `@unchecked Sendable` **purely to stop Swift 6 strict
concurrency from rejecting it**. That annotation is a promise to the compiler that you
synchronised the type yourself. Here the promise is a lie, on purpose. Recognising that
annotation as a red flag in review is part of the exercise.

---

## Reproduce the failure

Two builds. The plain one shows the *consequence*; the sanitized one is the *evidence*.

```bash
cd exercises/02-data-race

# 1. the consequence — run it twice and compare
swiftc -swift-version 6 -Onone broken/stats.swift -o /tmp/stats_broken
/tmp/stats_broken
/tmp/stats_broken

# 2. the evidence
swiftc -swift-version 6 -Onone -g -sanitize=thread broken/stats.swift -o /tmp/stats_tsan
/tmp/stats_tsan ; echo "exit status: $?"
```

Expected — note that the two plain runs disagree with each other:

```
EX02 build=broken threads=8 perThread=200000 expectedChunks=1600000 observedChunks=493500  lostChunks=1106500 ... correct=false
EX02 build=broken threads=8 perThread=200000 expectedChunks=1600000 observedChunks=575481  lostChunks=1024519 ... correct=false

WARNING: ThreadSanitizer: Swift access race (pid=19324)
WARNING: ThreadSanitizer: data race (pid=19324)
...
SUMMARY: ThreadSanitizer: Swift access race stats.swift in DownloadStats.record(bytes:)
ThreadSanitizer: reported 6 warnings
exit status: 134
```

**Timeout and safety.** Fixed iteration counts, no blocking waits, and a 60-second
watchdog that calls `_exit(75)`. Nothing here can hang. Exit status **134** from the
sanitized build is `SIGABRT`: once Thread Sanitizer has reported a warning it aborts at
exit rather than returning 0. That is why a sanitized CI job fails loudly on a race even
when the program's own output looks plausible.

---

## The evidence to collect

Keep these two things apart in your head, because an interviewer will ask you to:

| | Detector evidence | Lost-update output |
| --- | --- | --- |
| What it is | `WARNING: ThreadSanitizer: data race` plus the two conflicting stacks | `observedChunks` lower than `expectedChunks` |
| Reproducible? | **Yes** — every run, same finding | **No** — a different number every run |
| Proves what? | Two threads accessed the same memory with no ordering between them | Something is wrong, somewhere |
| Can it be absent while the bug is present? | Yes — TSan only sees code paths that actually execute | Yes — the interleaving may not lose anything on a given run |

Swift produces **two report kinds** for this one defect: `Swift access race` (an
exclusivity violation) and `data race` (the memory conflict). Engineers who have only seen
TSan's C output do not recognise the first one.

**Now try the third build** — and be ready to explain it:

```bash
swiftc -swift-version 6 -O broken/stats.swift -o /tmp/stats_O
/tmp/stats_O ; /tmp/stats_O ; /tmp/stats_O
```

On the reference machine this prints `observedChunks=400000` — *the same wrong number
every single time*. The optimiser collapsed each thread's 200,000 increments into one
addition, so the race went from many small ones to a few large ones. The output became
perfectly reproducible and stayed perfectly wrong.

**That is the answer to "I ran it twenty times".** A stable number is not a correct
number, and an unsynchronised program's behaviour is a property of the build, not of the
source.

---

## Success criteria

- [ ] The broken build still loses updates — you have not edited it.
- [ ] Your fixed build prints `correct=true` and `lostChunks=0` on two consecutive runs.
- [ ] Built with `-sanitize=thread`, your fixed build produces **no** `WARNING:
      ThreadSanitizer` line and exits 0.
- [ ] You did not reach for `@unchecked Sendable`.
- [ ] You can state why one lock per counter would be worse than one lock for both.

Run `./check.sh` to have all of that checked for you. Add `--quick` to skip the sanitizer
builds.

---

## Hints

<details>
<summary>Hint 1 — what the hardware actually does</summary>

`bytesReceived += bytes` compiles to a load, an add and a store. Two threads can both
load the value 41, both add, and both store 42. One increment is simply gone. Nothing in
the source marks where that can happen, which is why you need a tool rather than a
careful read.
</details>

<details>
<summary>Hint 2 — what has to become indivisible</summary>

The unit that must be indivisible is not the assignment. It is **the whole
read-modify-write**, and in this type it is *both* counters together, because a reader
should never see a byte total that does not match its chunk total.
</details>

<details>
<summary>Hint 3 — let the compiler help</summary>

If the repair is right, `@unchecked Sendable` becomes unnecessary and the type can be
plain `Sendable`. Deleting `@unchecked` and getting a clean build is itself a check on
your work: you have moved from asserting safety to having it verified.

`Mutex` from the `Synchronization` module (macOS 15+) is the direct tool. `NSLock` and
`OSAllocatedUnfairLock` also work; an `actor` works but changes every call site to
`await`. A serial `DispatchQueue` works and costs more.
</details>

---

## Solution

<details>
<summary>Reveal the solution</summary>

### The repair

```swift
final class DownloadStats: Sendable {
    private struct Totals { var bytes = 0; var chunks = 0 }
    private let totals = Mutex(Totals())

    func record(bytes: Int) {
        totals.withLock { t in
            t.bytes += bytes
            t.chunks += 1
        }
    }

    func snapshot() -> (bytes: Int, chunks: Int) {
        totals.withLock { ($0.bytes, $0.chunks) }
    }
}
```

Three things changed and each one matters:

1. **The counters moved inside the lock.** `Mutex` owns them; `withLock` is the only door.
   There is no longer a way to touch either counter without holding the lock, so this is
   not a convention that a future reader can quietly break.
2. **Both counters are in ONE lock.** Two independent locks would make each counter
   individually correct while still letting `snapshot()` return a byte total and a chunk
   total from different moments. The invariant spans both fields, so the critical section
   must too.
3. **`@unchecked Sendable` is gone.** `Mutex` is `Sendable` on its own terms, so the
   compiler now checks this type instead of taking our word for it.

What did **not** change: the workload, the thread count, the iteration count. Only the
boundary around the mutation moved.

### The distinction to state explicitly

A **data race** is about memory: two threads touch the same location with no ordering
between them and at least one writes. It is undefined behaviour, and Thread Sanitizer
finds it.

A **race condition** is about invariants: the program's logic depends on an ordering it
has not established. Every individual access can be perfectly locked and the program can
still be wrong.

Fixing the data race here does not automatically fix every race condition in the system —
it only makes the counters correct. If some other code reads `snapshot()`, decides "we are
under budget", and then records more, that check-then-act is a race condition your lock
does nothing about, and Thread Sanitizer will be silent about it.

### What Thread Sanitizer cannot do

- It is a **runtime** detector, not a static one. It reports races on code paths that
  actually executed, with the interleavings that actually happened.
- Apple documents roughly **5–10× memory** and **2–20× slowdown** under TSan, so it is a
  CI and debugging tool, not something you ship.
- It does **not** see race conditions, only data races. Silence is not proof of
  correctness.
- Its findings are real, though: unlike a lost-update count, a TSan report is not a
  probabilistic observation.

### What the interviewer is listening for

- That you named the detector rather than the symptom.
- That you distinguished data race from race condition without being prompted.
- That you noticed `@unchecked Sendable` and treated it as the smell it is.
- That you had an answer for "but it prints the right number at `-O`".

</details>

---

## Limitations

- The lost-update counts here are **nondeterministic by nature**. The bundled check
  asserts only that the total is never *higher* than the true one and that at least one of
  two runs lost something. It never asserts a figure, because asserting a figure would be
  asserting a race.
- The `-O` behaviour is a property of **this compiler version** on this machine. Another
  version may collapse the loop differently, or not at all. The lesson — that the build
  configuration changes the observed behaviour of an unsynchronised program — is general;
  the specific number is not.
- `Mutex` requires macOS 15 or later. On older deployment targets use
  `OSAllocatedUnfairLock` or `NSLock`, with the same "the lock owns the data" discipline.
