phongn opened a new issue, #13572:
URL: https://github.com/apache/trafficserver/issues/13572

   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.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to