JosiahWI commented on PR #13170: URL: https://github.com/apache/trafficserver/pull/13170#issuecomment-5375220818
A general Treiber stack is thread safe for access to the data because the data is part of the stack node. This might be the case for `InkAtomicList` - I would have to review it again. For `InkFreeList` specifically, the general Treiber stack reasoning breaks down, because the _data does not live on the stack_. What lives on the stack are stack bookkeeping objects which are overwritten with some other object by an `Allocator`. That ends the lifetime of the bookkeeping metadata. Reading the version out of the freelist node, even through a `std::atomic_ref`, is a data race. But it's more than a data race. It's also very likely to be an aliasing violation, because the memory the freelist is reading from may be in the middle of some subobject in an `HttpHdr` or what have you, and the effective type may no longer be the same as the freelist node version tag. The version tag object no longer exists. Whether or not it's acceptable to leave that in the program anyway may be an interesting question, but I do not think there is a doubt as to whether or not it is UB. It is UB. And a comparison to a general Treiber stack is a subtle pitfall - for the `InkFreeList` case, the properties that make a Treiber stack thread safe do not all hold. The `InkAtomicList` case may be worth another look - if no one touches the stack metadata of a popped node, then the correctness does hold according to the Treiber stack design. -- 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]
