DEV Community

Cover image for Four Bugs in Fourteen Lines
ISNDEV
ISNDEV

Posted on

Four Bugs in Fourteen Lines

Lock contention, starvation, false sharing, cache misses. They're not four bugs. They're one decision, made on day one, that nobody wrote down.


Here is a class you have written. Maybe not this one exactly, but one shaped like it.

class SessionRegistry {
    std::mutex                       mu_;
    std::unordered_map<u64, Session> sessions_;
    std::atomic<uint64_t>            hits_{0};
    std::atomic<uint64_t>            misses_{0};

public:
    Session *find(u64 id) {
        std::lock_guard lock(mu_);
        auto it = sessions_.find(id);
        if (it == sessions_.end()) { misses_.fetch_add(1); return nullptr; }
        hits_.fetch_add(1);
        return &it->second;
    }

    void refresh(u64 id) {
        std::lock_guard lock(mu_);
        auto &s = sessions_[id];
        s.data  = db_.query(id);        // network round trip
        s.stamp = now();
    }
};
Enter fullscreen mode Exit fullscreen mode

Fourteen lines. It compiles clean, passes review, passes tests, and ships. Nobody wrote it carelessly. Every line is the obvious line.

It also contains, simultaneously, all four of the classic multithreading pathologies.

Lock contention. One mutex, every lookup. Twelve worker threads funnel through a single serialization point on the hottest path in the service. Under load, your profile stops being about your code and starts being about the futex.

Starvation. db_.query() is a network round trip, and it runs while the lock is held. That's not milliseconds of critical section — that's milliseconds times however unlucky the network feels. Every reader in the process queues behind one writer waiting on a socket. The reviewer didn't catch it because the blocking call is one level down, behind a member function that looks like an accessor.

False sharing. hits_ and misses_ are eight bytes each, declared adjacently, almost certainly in the same 64-byte cache line. They share nothing logically. Every core that increments one invalidates the other's line on every other core. Two counters nobody reads until the metrics endpoint is hit, generating coherence traffic on every single lookup.

(They also don't need to be atomic. They're only ever touched under the lock. Everyone writes them that way anyway, because std::atomic on a counter has become a reflex.)

Cache misses. find returns a raw pointer into a map, after releasing the lock — which is separately a lifetime bug, since a concurrent refresh can rehash and invalidate it. But set that aside. The session's cache lines get pulled into whichever core the thread pool happened to pick this time. Next request for the same session lands on a different core, and the lines migrate again. The working set never settles anywhere.

Now, each of these has a well-known fix. Shard the mutex. Move the query outside the critical section. Pad the counters to a cache line. Pin the threads, or add a per-thread cache in front.

Four fixes, four pull requests, four benchmarks proving each one helped. All of them real. And all of them applied after the architecture already made the mistake.

Because these aren't four bugs. They're one decision, made implicitly on the first day of the project and never written down anywhere:

More than one thread is allowed to reach this memory.

Once that's true, everything downstream is damage control. You don't prevent contention, you negotiate it. You don't prevent false sharing, you pad around it. You don't get locality, you fight for it against a scheduler that doesn't know what your data means. The bug isn't in SessionRegistry. It's one level up, in a decision that never appeared in a review.

I got tired of patching the symptom. So I wrote a runtime around the opposite decision.

The rule

One worker thread per lane. One event loop on that thread. Actors, sockets, timers and coroutine resumptions all live on it. Mutable state belongs to exactly one lane. To touch state on another lane, you send a message.

The runtime is qb — C++20, Apache-2.0, mine. A lane is a VirtualCore: one worker thread that owns a set of actors, runs a single event loop over them, and drains one private lock-free mailbox.

An actor is thread-affine to the VirtualCore that created it and never migrates, for its entire lifetime. That one invariant is what the rest of this article is about, because it's what turns the four pathologies from discouraged into unavailable.

SessionRegistry under that rule isn't a fixed version of the class above. It's a class with no mutex, no atomics, and a plain std::unordered_map, because there is no second thread that could reach it.

Going back through the four

I want to be precise, including where the honest answer is "partly."

Contention: eliminated on actor state. Not reduced — absent. There's no lock because there's no second thread that can reach the memory. The fields are ordinary single-threaded C++ variables that happen to live inside a concurrent program. The compiler can keep them in registers across a handler. No acquire/release fence bracketing every access, no coherence traffic from the lock word itself.

Starvation: reframed, not deleted. Nobody waits on a lock, so lock-based starvation is gone. But actors on one lane share a worker, and that worker runs one handler at a time. A slow handler starves its lane-mates. The failure moved from unpredictable, depends on acquisition order to predictable, depends on placement, and shows up in a profile as one named handler. That's a real improvement. It is not a free lunch, and I come back to it in the costs.

False sharing: eliminated between lanes, still yours inside one. Two actors on two cores cannot false-share their hot state, because there is no shared state to accidentally colocate. What's left is the inter-core ring buffer — framework code, which is where the padding belongs. What is not solved: false sharing inside one actor's own structures, or between an actor and something foreign you handed it. The model shrinks the surface. It does not align your structs for you.

Cache locality: this is the real prize. Actor state is touched by exactly one thread, on one core, for the actor's whole life. It stays hot in that core's L1/L2 instead of being dragged around by a scheduler that has no idea what it is. No migration, no cold refill. No sharing, no coherence traffic. And because one lane processes a run of similar events in sequence, the code path stays hot alongside the data.

You can make it deliberate, too. CoreInitializer::setAffinity pins the worker to physical CPUs — pthread_setaffinity_np on POSIX, SetThreadAffinityMask on MSVC. Note best effort: a failed pin logs a warning and startup continues, and logical core ids are not CPU numbers. engine.core(7) is logical core 7; it lands on physical CPU 7 only if you ask.

Branch prediction: plausible, and the one I'd argue about. The story is that dispatching many events of one type to one actor keeps the predictor warm and the i-cache hot, versus a pool where consecutive tasks are arbitrary. I believe it. I haven't isolated it from the cache effects in a way that would convince me if someone else claimed it, so I'm listing it as a hypothesis, not a benefit. If you've measured this properly on a shard-per-core system, I'd like to see it.

There is exactly one multithreaded surface, and it isn't yours

None of this makes concurrency disappear. It concentrates it into one place.

In qb, the only memory that multiple threads concurrently mutate is the inter-core mailbox layer. Each VirtualCore consumes from exactly one inbound mailbox — a multi-producer, single-consumer lock-free ring. Many cores enqueue; only the owner ever dequeues.

Everything an actor can see is single-thread-per-core with no synchronization whatsoever. The actor maps, the service-id pool, the thread_local event loop, the coroutine scheduler: one thread each, not one atomic among them.

That's the actual trade. The hard concurrent code doesn't go away — it gets written once, in one file, and never again in application code. Routing is O(1): a destination core resolves to a mailbox slot through a precomputed dense index, no hash lookup. Each loop iteration flushes outbound events to peer mailboxes, then drains its own.

What I like about this framing is that it's falsifiable. If someone finds a second place where application-visible state is concurrently mutated, that's a bug in the runtime — not a design discussion.

Owning state doesn't teach you how to wait

Share-nothing tells you who may mutate what. It says nothing about waiting.

An actor can still do this, and it's worse than the mutex was:

auto result = blocking_database_query();
Enter fullscreen mode Exit fullscreen mode

Ten actors, three sockets and a timer are now stuck behind that call. You preserved state ownership and destroyed progress. At least the mutex only blocked contenders.

So I/O has to be non-blocking, and dependent operations have to stay readable. That's what C++20 coroutines do here:

auto reply = co_await database.execute("select_user", params);
Enter fullscreen mode Exit fullscreen mode

At the suspension point the frame holds the operation's state, the stack unwinds to the scheduler, and the lane goes elsewhere — dispatches another actor, services a socket, fires a timer, resumes a different coroutine. When the reply lands, the coroutine resumes on the same lane. Same thread, same cache, same ownership boundary. No thread was spawned because you typed co_await, and no continuation was handed to a foreign executor.

That last clause is the entire reason to put the coroutine scheduler on the event loop rather than beside it.

What it looks like

Smallest version:

struct GreetingEvent : qb::Event {
    qb::string<64> message;
    explicit GreetingEvent(const char *msg) : message(msg) {}
};

class GreeterActor : public qb::Actor {
public:
    qb::io::async::task<bool> onInit() final {
        registerEvent<GreetingEvent>(*this);
        push<GreetingEvent>(id(), "Hello");
        co_return true;
    }

    void on(const GreetingEvent &event) {
        qb::io::cout() << "Received: " << event.message << '\n';
        kill();
    }
};

int main() {
    qb::Main engine;
    engine.addActor<GreeterActor>(0);
    engine.start();
    engine.join();
}
Enter fullscreen mode Exit fullscreen mode

No mutex, no condition variable, no shared queue — not avoided, just not applicable.

And a handler crossing three protocols:

qb::io::async::task<void>
TaskManager::handle_list_tasks(ctx_t ctx) {
    auto cached = co_await _redis.get("tasks:list");

    if (cached.ok() && cached.result().has_value() && !cached.result()->empty()) {
        ctx->response().add_header("X-Cache", "HIT");
        ctx->json(qb::json::parse(*cached.result()));
        co_return;
    }

    auto result = co_await _db->execute("select_all_tasks", qb::pg::params{});

    if (!result.ok()) {
        ctx->internal_server_error(result.error().what());
        co_return;
    }

    auto json = models::TaskList(result.result(), false).to_json();
    (void) co_await _redis.setex("tasks:list", 60LL, json.dump());

    ctx->response().add_header("X-Cache", "MISS");
    ctx->json(json);
}
Enter fullscreen mode Exit fullscreen mode

HTTP session, Redis client, Postgres client, coroutine frame and actor: one VirtualCore. Three protocols, one execution context, zero affinity handoffs. While Redis or Postgres is pending the frame is suspended and the lane keeps serving everyone else.

The Postgres and Redis clients speak their wire protocols directly rather than wrapping blocking libraries. That's the only way this property survives contact with a real database.


The bill

If I stopped here this would be marketing. Here's what the rule costs.

One blocking call poisons an entire lane. Architectural, not advisory. A blocking syscall or a long CPU loop delays every actor, socket, timer and coroutine on that worker. qb caps the ready-coroutine drain at 65,536 resumptions per loop turn so the scheduler queue can't monopolize a turn, but it does not preempt a running handler and enforces no per-task CPU budget. Your handler starves the lane until it returns. Heavy CPU work has to be chunked around explicit yields, isolated on its own lane, or pushed out through a bridge.

No work stealing. Bad placement stays bad. Actors never migrate, so a hot actor pins its lane while a neighbour idles and nothing rebalances it. Placement is also an interference domain: a latency-critical actor colocated with a throughput-heavy one inherits that neighbour's scheduling delay even when nobody blocks and nobody misuses the API. Colocation buys locality and sells coupling. This model wants a natural partitioning key — account, session, instrument, tenant, symbol. Without one, a work-stealing pool wins.

An uncaught exception in a handler kills the process. The typed dispatch trampoline is noexcept, so a throw escaping on(Event&) calls std::terminate. It does not kill and restart that one actor. A lane is a failure domain, and actor semantics do not hand you Erlang-style isolation for free.

Cross-core transport relocates raw bytes, which is an ABI bet. Events move between buffers by relocating bytes; the source is abandoned without running a destructor there. Trivially copyable types aren't the issue — qb also treats selected non-trivially-copyable representations as bitwise relocatable, and C++20 gives you no portable operation for that. WG21 P2786 exists precisely because byte-moving such an object violates the object model. So this is empirical behaviour on the compiler and standard-library combinations qb targets, not a guarantee from the abstract machine, and I don't want it mistaken for one. By-value std::string is rejected by contract: libstdc++'s short-string form stores a pointer into the object's own inline buffer, so relocating it yields a self-pointer into dead storage. Use plain data or qb::string<N>. A debug scan catches the common cases on the cross-core hop; it can't prove correctness.

The routing topology costs memory, quadratically. Outbound pipes and inbound rings are allocated eagerly across the configured core set and never shrink: roughly 0.6 MiB at one core, 6.3 at four, 22.5 at eight, 85 MiB at sixteen. Values are build-dependent; the shape isn't. That's the price of avoiding shared cursors and steady-state allocator traffic, and you should budget it before configuring a lot of lanes.

The default burns a CPU while idle. setLatency(zero) — the default — is busy-spin: the loop never blocks and holds its core at 100% for minimum wake-up delay. With a non-zero latency, recent activity earns a spin credit and then the worker parks on a condition variable until a peer notifies it. Lower idle CPU, possible wake-up delay. A market-data process and an admin service shouldn't make the same choice, and the runtime can't make it for them.

Sequential handling is not global ordering. Ordered push preserves FIFO only from one source actor to one destination actor. Independent senders, different destinations and broadcasts have no global order. Absence of data races does not imply deterministic distributed behaviour, and conflating the two is an excellent way to ship a subtle bug.

Why there's no benchmark number here

The performance story is obvious: no synchronization on lane-local state, hot caches, O(1) routing, batched message movement, no executor hop for async I/O.

That's a list of properties, not a result. Messages-per-second means nothing without hardware and topology, compiler and flags, actor and core counts, the same-core versus cross-core mix, payload size, allocation behaviour, how much real work each handler does, and latency percentiles next to throughput. A number without those is decoration.

So I left it out rather than post one that flatters my own design. Name a configuration you'd find convincing and I'll run it and publish the method with it.

What I actually think

This shouldn't be the execution model for most C++ applications. If your work is independent and CPU-bound, a work-stealing pool is better. If your critical dependencies only expose blocking APIs, you'll fight this daily. If your state genuinely must be mutated from arbitrary threads, no amount of discipline makes this the right tool.

But go back to those fourteen lines. Nothing in SessionRegistry is exotic, and nothing in it is stupid. It's the default outcome of an execution model where any thread may touch any memory — and we've collectively agreed to treat that as a law of nature instead of what it is.

It's a choice. Making the other one costs you work stealing, transparent placement, and a hard rule about blocking. That's the trade, stated plainly, and I'd rather argue about the trade than about the pitch.

So: where does this break down? Which workload of yours would this have made worse, and by how much? I'm more interested in that than in another star.


Code: github.com/isndev/qb — C++20/23, Linux/macOS/Windows, x86_64 and ARM64, Apache-2.0. If you read one page, read the threading model.

Top comments (0)