cmcfarlen commented on PR #13583: URL: https://github.com/apache/trafficserver/pull/13583#issuecomment-5527960347
Moved in f0478d5819. Worth noting the mechanism as described is not quite what was happening. Binding `std::string &cur = std::get<0>(std::get<0>(*blob)[offset])` is address computation; it does not access the string's bytes. The first actual read was `cur.length()`, which was already inside the lock, so there was no unsynchronized access to the string and no data race by the standard's definition. The change is still worth making. Computing that reference outside the lock made the critical section look wider than it was, and nothing in this function has a reason to sit outside it — the name is the key `_lookups` is indexed by, so the whole replacement belongs in one place. The allocation check stays outside, since it reads only the published bound and is lock free like its neighbours. Low risk either way: `rename()` has no callers outside the tests. `LogObject::rename` is an unrelated class. Full `test_tsutil` and `test_records` pass; whole tree builds. -- 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]
