apache / apache/trafficserver

InkAtomicList empty-checks and head peeks bypass atomic loads

Open
#13,572 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
2k
Forks
874
Avg merge
6d 15h
Merged PRs (30d)
46

Description

While working on #13555 (see also #13571) we audited all accesses to the lock-free list head (`head_p`). All mutations go through `INK_QUEUE_LD` / `ink_atomic_cas`, but a few read-only "peeks" use plain, non-atomic reads of the pointer field:

* `INK_ATOMICLIST_EMPTY` (`include/tscore/ink_queue.h`), used by `ProtectedQueue::wait` (`src/iocore/eventsystem/ProtectedQueue.cc`) to decide whether to `cond_timedwait`.
* `AtomicSLL::head()` and `AtomicSLL::empty()` (`include/tscore/List.h`).
* `LogObject` constructor/destructor reads of `m_log_buffer` (`src/proxy/logging/LogObject.cc`), though these run in single-threaded contexts.

### Impact

These reads race with concurrent CAS writers, which is formally undefined behavior (and visible to TSAN). In practice the impact is bounded today: the pointer half is a single aligned 8-byte access on all supported targets, so it does not tear, and the one wait-gating use is a *timed* wait, so a stale answer costs at most one timeout period of event latency, not a lost wakeup.

### Suggested fix

Convert the peeks to `__atomic_load_n(&head.s.pointer, __ATOMIC_RELAXED)` (or an inline helper next to `INK_QUEUE_LD`). This is free on every supported platform and removes the formal race without changing behavior.

Low priority; filing so the audit result does not get lost.

Contributor guide

Open the contributing guide

Research direction

Start by reading INK_QUEUE_LD in include/tscore/ink_queue.h, then inspect INK_ATOMICLIST_EMPTY, AtomicSLL::head(), AtomicSLL::empty(), and the m_log_buffer accesses in the listed source files. Update the read-only head peeks to use relaxed atomic loads, and verify that the relevant checks no longer produce the reported race under TSAN without changing behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.