Copilot commented on code in PR #13616:
URL: https://github.com/apache/trafficserver/pull/13616#discussion_r3917900139
##########
include/tsutil/Metrics.h:
##########
@@ -364,6 +469,30 @@ class Metrics
return (id >= 0 && ((blob < _cur_blob && entry < MAX_SIZE) || (blob ==
_cur_blob && entry <= _cur_off)));
}
+
+ /** Whether @a id names a slot that @c create has actually written.
+ *
+ * Stricter than @c valid in two ways, both of which matter when the id
can be manufactured
+ * rather than handed out by the store. The offset is the low 16 bits of
the id, so it can name
+ * a slot well past @c MAX_SIZE in a blob that is full; indexing on that
runs off the end of the
+ * blob. And @c _cur_off is the *next* free slot, not the last used one,
so @c valid accepts one
+ * slot that does not exist yet -- writing a flag there would be inherited
by whatever metric is
+ * created in it later.
+ *
+ * The blob index needs no range check of its own: @c _splitID has already
masked it to less
+ * than @c MAX_BLOBS, per the static assertion on that mask above.
+ */
+ bool
+ allocated(IdType id) const
+ {
+ auto [blob, entry] = _splitID(id);
+
+ if (id < 0 || entry >= MAX_SIZE || !_blobs[blob]) {
+ return false;
+ }
+
+ return blob < _cur_blob || (blob == _cur_blob && entry < _cur_off);
Review Comment:
allocated() reads `_blobs[blob]`, `_cur_blob`, and `_cur_off` without
holding `_mutex` (or using atomics), but those fields are mutated under
`_mutex` in `create()` / `addBlob()` / `createSpan()`. If
`unlist()`/`relist()`/`listed()` are called concurrently with metric
registration, this is a C++ data race (UB) and could observe a transient
`_blobs[blob]` state while a new blob is being installed.
Consider either (a) taking `_mutex` inside `allocated()` (and any callers
that rely on it), or (b) making the relevant fields atomically readable (and
ensuring blob publication is also race-free) so the intended lock-free checks
are well-defined. The current approach relies on unsynchronized reads of shared
state.
--
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]
