Common mistakes with threads

Overview

You already know how to write thread-safe code (guard shared state so concurrent access is well-defined) and you understand deadlocks (two threads each waiting for a lock the other holds). This lesson is the field guide that ties those ideas to the specific mistakes people actually make the first dozen times they touch pthreads. Each mistake here is a concrete misuse of the primitives you have already met — a missing lock, a lock held too long, a pointer that outlives the memory it points at.

The goal is pattern recognition. Most threading bugs are not exotic; they are the same handful of errors wearing different clothes. Once you can name them, you catch them in review instead of in production at 3 a.m.

Why it matters

Threading bugs are the worst kind: they are timing-dependent, so they pass every test on your laptop and then corrupt data once a week on a busy server. A single unprotected counter++ can silently lose updates in a billing system; a lock held across a network read can freeze an entire request pool. In security-sensitive code, races create TOCTOU windows — a value is validated, then changed by another thread before it is used. Learning to recognize these patterns up front is far cheaper than debugging a heisenbug that only appears under load.

Core concepts

The shape of every threading bug

Almost every pthreads mistake is one of four shapes. Keep this table in your head:

Mistake 1 — passing &i from a loop

This is the number-one beginner trap. You start threads in a loop and hand each one the address of the loop counter:

for (int i = 0; i < N; i++)
    pthread_create(&t[i], NULL, work, &i);   /* WRONG */

Every thread receives the same address — &i. The threads do not run instantly; by the time they dereference the pointer, the loop has moved on, so they all read whatever i happens to be now (often N). You wanted the values 0,1,2,...; you get garbage.

  main's stack:  [ i ]  <-- one box, address &i
                   ^  ^  ^
  thread0 -------- |  |  |     all three point at the SAME box
  thread1 ------------ |  |
  thread2 --------------- |

The fix is to give each thread its own storage. Either pass a small integer by value through the void* (via (void*)(intptr_t)i, unpacked with (int)(intptr_t)arg), or malloc a struct per thread and pass that pointer. The heap approach scales to real arguments.

Mistake 2 — racing on shared state

counter++ reads like one operation but compiles to three: load, increment, store. Two threads can both load 41, both compute 42, and both store 42 — one increment vanishes. That is a data race, and in C a data race is undefined behavior, not just a wrong number.

  thread A        thread B
  load  41
                  load  41
  add -> 42
                  add -> 42
  store 42
                  store 42     <-- one update lost

Two fixes. For a single scalar, _Atomic long counter; makes counter++ an indivisible operation. For anything more than one scalar — a struct, a linked list, two fields that must agree — use a mutex around the whole read-modify-write.

Knowledge check: You protect counter++ with a mutex in one function but another function writes counter directly without the lock. Is the program safe?

Answer

No. A mutex only helps if every access to the shared data takes the same lock. One unlocked writer reintroduces the race. The lock protects data by convention, not by force — the compiler will not stop you from touching the data without it.

Mistake 3 — holding a lock across blocking I/O

A critical section should be as short as possible. If you hold a mutex while doing read(), printf(), or a network call, every other thread that needs that lock stalls for the whole duration of the I/O. Under load this serializes your whole program behind the slowest operation.

Copy what you need out of shared state under the lock, unlock, then do the slow work:

pthread_mutex_lock(&m);
Job job = queue_pop(&q);      /* fast: touch shared state */
pthread_mutex_unlock(&m);
process(job);                 /* slow: no lock held */

Mistake 4 — inconsistent lock ordering (deadlock)

You met this in the prerequisite. If path 1 locks A then B, and path 2 locks B then A, they can each grab one and wait forever for the other. The cure is a single global rule: whenever you need both, always lock them in the same order (for example, by address). This lesson's job is just to remind you that deadlock hides inside ordinary-looking code — anywhere two locks are ever held at once.

Mistake 5 — if instead of while around cond_wait

Condition variables can wake spuriously — the thread returns from pthread_cond_wait even though nothing changed. Worse, another thread may have consumed the resource between the signal and your wakeup. So you must re-test the predicate in a loop:

while (!ready)                          /* while, never if */
    pthread_cond_wait(&cv, &m);

Mistake 6 — returning a stack address from the thread

When a thread function returns, its stack frame is gone. Returning &local (or storing &local where the joiner reads it) hands out a dangling pointer. Return a value packed into the void*, or a pointer to heap memory the joiner will free — as this lesson's example does.

Mistake 7 — forgetting to join or detach

Every joinable thread holds resources (its TCB) until someone pthread_joins it. If you neither join nor detach, you leak per-thread memory for the life of the process. Pick one: join when you need the result or must wait; detach (pthread_detach) for fire-and-forget workers.

Syntax notes

int pthread_create(pthread_t *thread, const pthread_attr_t *attr,
                   void *(*start)(void *), void *arg);

• thread: out-param, receives the thread id. attr: NULL for defaults. start: the function the thread runs. arg: the single void* passed to it — make it point at per-thread storage.
• Returns 0 on success, or an errno value on failure (it does not set errno or return -1). Always check rc != 0.

int pthread_join(pthread_t thread, void **retval);

• Blocks until thread finishes. retval (may be NULL) receives whatever the thread returned. Joining also frees the thread's resources — a joinable thread you never join leaks.

int pthread_detach(pthread_t thread);

• Marks a thread so its resources are auto-reclaimed on exit. You can no longer join it. Use for fire-and-forget work.

int pthread_mutex_lock(pthread_mutex_t *m);
int pthread_mutex_unlock(pthread_mutex_t *m);

• Acquire/release a mutex. Statically initialize with PTHREAD_MUTEX_INITIALIZER. Every access to the guarded data must take the same lock. Keep the region between lock and unlock small.

_Atomic long counter;   /* <stdatomic.h> — counter++ is now indivisible */

• For a single scalar shared across threads, an atomic type removes the race without a mutex.

Build note: always compile and link threaded code with -pthread (or -lpthread). Missing it can produce link errors or, on some platforms, silently broken locking.

Lesson

Here are eight mistakes that show up again and again in threaded C programs. Each one has a clear cause and a clear fix.

1. Forgetting -pthread

Leave this flag off and you get a linker error — or worse. On some platforms the code compiles, but pthread_mutex calls silently turn into no-ops, so your locking does nothing.

Always compile and link with -pthread.

2. Passing &i from a loop

If you start threads inside a loop and pass the address of the loop variable i, every thread ends up reading the same memory. By the time the threads run, i already holds its post-loop value, so they all see it.

Pass the value itself, or pass a pointer to memory allocated on the heap for each thread.

3. Race on a global counter

counter++ looks like one step, but it is really three: read, add, write. Two threads can interleave these steps and lose updates. This is a data race.

Protect the counter with a mutex, or declare it as _Atomic long.

4. Holding a mutex across a blocking I/O call

If a thread keeps a lock held while it waits on slow I/O, every other thread that needs that lock stalls too.

Release the lock before doing I/O.

5. Locking in different orders

If one path locks A then B, and another locks B then A, the two will eventually collide and wait on each other forever. This is a deadlock.

Adopt a single global rule for the order in which locks are acquired, and follow it everywhere.

6. Using signal() in a threaded program

The old signal() function has poorly defined behavior with threads.

Use sigaction() instead. Consider blocking signals on worker threads with pthread_sigmask.

7. Using if instead of while around cond_wait

A condition variable can wake a thread even when nothing changed — a spurious wakeup. If you check the condition with if, the thread continues on a false assumption.

Always re-check the condition in a while loop.

8. Returning a stack address from the thread function

The thread's stack disappears the moment the thread exits. Any pointer into it becomes invalid.

Return a heap pointer or a value, never the address of a local variable.

Code examples

#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>

#define NTHREADS 8
#define BUMPS    100000

/* Shared state guarded by one mutex. */
static long counter = 0;
static pthread_mutex_t counter_lock = PTHREAD_MUTEX_INITIALIZER;

/* Each thread gets its OWN heap-allocated argument, so there is no
   aliasing of a shared loop variable. */
typedef struct {
    int id;
    long local_sum;   /* filled in by the thread, read after join */
} task_t;

static void *worker(void *arg)
{
    task_t *t = arg;              /* recover our private argument */
    long mine = 0;

    for (int i = 0; i < BUMPS; i++) {
        /* Critical section: read-modify-write of the shared counter.
           Keep it tiny and do NO blocking I/O while holding the lock. */
        pthread_mutex_lock(&counter_lock);
        counter++;
        pthread_mutex_unlock(&counter_lock);
        mine++;
    }

    t->local_sum = mine;         /* stash a result the joiner can read */
    return t;                    /* return the heap pointer, never &local */
}

int main(void)
{
    pthread_t th[NTHREADS];
    task_t   *args[NTHREADS];

    for (int i = 0; i < NTHREADS; i++) {
        args[i] = malloc(sizeof *args[i]);   /* per-thread storage */
        if (!args[i]) { perror("malloc"); return 1; }
        args[i]->id = i;
        args[i]->local_sum = 0;

        int rc = pthread_create(&th[i], NULL, worker, args[i]);
        if (rc != 0) {                        /* pthreads return an errno, not -1 */
            fprintf(stderr, "pthread_create: %d\n", rc);
            return 1;
        }
    }

    long grand_total = 0;
    for (int i = 0; i < NTHREADS; i++) {
        void *ret;
        pthread_join(th[i], &ret);            /* wait, then reclaim resources */
        task_t *t = ret;
        grand_total += t->local_sum;
        free(t);                              /* we allocated it, we free it */
    }

    printf("counter      = %ld\n", counter);
    printf("grand_total  = %ld\n", grand_total);
    printf("expected     = %ld\n", (long)NTHREADS * BUMPS);
    printf("%s\n", counter == (long)NTHREADS * BUMPS ? "OK: no lost updates"
                                                     : "BUG: race detected");
    return 0;
}

Line by line

• static long counter + PTHREAD_MUTEX_INITIALIZER: the shared state and the one lock that guards it, both statically initialized so no runtime setup is needed.
• typedef struct { int id; long local_sum; } task_t;: the per-thread argument. Because each thread gets its own task_t, there is no aliasing — this is the fix for the &i-from-a-loop mistake.
• task_t *t = arg; inside worker: unpack the void* back to the real type. Each thread's t points at a different heap block.
• pthread_mutex_lock … counter++ … pthread_mutex_unlock: the critical section is exactly one line long. Every increment goes through the same lock, so no update can be lost. Nothing slow happens while the lock is held.
• t->local_sum = mine; return t;: the thread records its own tally and returns the heap pointer — never &mine, which would dangle the instant the thread exits.
• args[i] = malloc(...) in the create loop: fresh storage per thread, filled before the thread starts. This is the safe alternative to passing &i.
• if (rc != 0): pthreads report errors by returning an errno, so we test the return value, not errno.
• pthread_join(th[i], &ret): wait for each thread, recover its returned pointer, add its tally, then free(t). Joining also reclaims the thread's resources, avoiding a leak.
• Final printfs: cross-check counter against the expected total. If the lock were missing, counter would come out less than 800000 under load.

Common mistakes

1. Passing the loop variable's address

for (int i = 0; i < N; i++)
    pthread_create(&t[i], NULL, work, &i);   /* WRONG */

Why it breaks: all threads share &i; they read i after the loop has changed it, so they see the same (usually final) value.

for (int i = 0; i < N; i++) {
    int *p = malloc(sizeof *p); *p = i;      /* per-thread copy */
    pthread_create(&t[i], NULL, work, p);    /* thread frees p */
}

2. Unprotected shared counter

counter++;                                    /* WRONG: data race */

Why it breaks: read-modify-write is three steps; interleaving loses updates and is undefined behavior.

pthread_mutex_lock(&m); counter++; pthread_mutex_unlock(&m);
/* or: _Atomic long counter; counter++; */

3. Lock held across blocking I/O

pthread_mutex_lock(&m);
fwrite(buf, 1, n, logfile);                   /* WRONG: slow, lock held */
pthread_mutex_unlock(&m);

Why it breaks: every other thread needing m stalls for the whole write.

pthread_mutex_lock(&m); Job j = pop(&q); pthread_mutex_unlock(&m);
fwrite(...);                                  /* do slow work unlocked */

4. Returning a stack address

void *work(void *_) { int r = compute(); return &r; }  /* WRONG */

Why it breaks: r's frame is destroyed when the thread exits; the joiner reads freed memory.

void *work(void *_) { return (void*)(intptr_t)compute(); } /* value */
/* joiner: int r = (int)(intptr_t)ret; */

Debugging tips

• ThreadSanitizer first. Compile with cc -fsanitize=thread -g and run. TSan reports the exact two accesses that race, with both stacks. It catches races that never once misbehaved in normal testing.
• Non-determinism is the tell. If output changes run to run, or only breaks under load or on a busier machine, suspect a race or missing sync. Add a usleep inside a suspected critical section to widen the window and make the bug reproducible.
• Deadlock diagnosis: if the program hangs, attach with gdb -p <pid>, then thread apply all bt. Two threads stuck in pthread_mutex_lock on different mutexes is the classic lock-ordering deadlock.
• Valgrind's helgrind/DRD are alternative race and lock-order detectors when TSan is unavailable.
• printf debugging in threads is itself a shared resource — interleaved output can mislead. Prefix each line with the thread id, or serialize logging behind its own lock, so you can read the timeline.

Memory safety

• Data races are undefined behavior, not merely wrong numbers. The compiler may cache a shared variable in a register or reorder loads/stores in ways that make an unlocked read see stale or torn values. volatile does not fix this — use a mutex or _Atomic.
• Argument lifetime: memory you pass to pthread_create must stay valid until the thread is done reading it. A malloc'd per-thread struct is safe; a pointer into a loop-scoped local is not.
• Return lifetime: never return or expose a pointer into the thread's stack. Return a value or heap pointer; the joiner owns and frees the heap pointer.
• Ownership after join: only after pthread_join returns is it safe to free memory the thread was using and to trust its results. Freeing while the thread may still touch it is a use-after-free.
• Leaks: a joinable thread that is never joined leaks its TCB. Join it, or create it detached.
• Mutex discipline: every access to guarded data must take the same lock. One stray unlocked access reintroduces UB even if every other access is perfect.

Real-world uses

Thread pools in web servers (nginx workers, database connection pools) live or die by these rules: tiny critical sections, no locks held across I/O, consistent lock order. Reference-counting in shared libraries and garbage-collected runtimes relies on atomic increments/decrements exactly like the counter here. Logging frameworks serialize writes behind a lock — and get it wrong by holding that lock across a slow flush. Best practice across all of them: keep shared mutable state small and few, prefer immutable or thread-local data, use atomics for lone counters, and put every threaded binary through ThreadSanitizer in CI before it ships.

Practice tasks

1. Reproduce the race. Delete the mutex around counter++ in the example, run the program many times (a shell loop), and observe counter coming out less than 800000. Note that it sometimes still prints the right answer — that is why races are dangerous.
2. Confirm with TSan. Recompile the racy version with -fsanitize=thread -g and read the report. Identify the two racing accesses in the output.
3. Fix without a mutex. Change counter to _Atomic long, remove the lock, and verify the total is correct again. Explain in a comment why this works for one scalar but not for a two-field struct.
4. Trigger and fix the &i bug. Rewrite the create loop to pass &i, observe the wrong per-thread ids, then fix it by passing an intptr_t value through the void*.
5. Build a deadlock, then break it. Add a second mutex and a worker that locks the two mutexes in the opposite order from another worker; make the program hang, capture it with gdb's thread apply all bt, then impose a global lock order to fix it.

Summary

• Most threading bugs are one of four shapes: aliasing, missing sync, bad sync, or bad lifetime.
• Give each thread its own argument storage — never pass &i from a loop.
• counter++ on shared data is a data race (undefined behavior); fix with a mutex or _Atomic, and make every access take the same lock.
• Keep critical sections tiny; never hold a lock across blocking I/O.
• Always lock multiple mutexes in a consistent global order to avoid deadlock.
• Use while, not if, around pthread_cond_wait (spurious wakeups).
• Never return a stack address from a thread; return a value or heap pointer, and join or detach every thread.
• Compile with -pthread, and run under ThreadSanitizer before shipping.

Practice with these exercises

• Forgotten joins
• Classify a threading bug
• Build a tiny thread-safe counter type
• Fix the race with a pthread_mutex_t