Copilot commented on code in PR #13608:
URL: https://github.com/apache/trafficserver/pull/13608#discussion_r3900114339
##########
src/iocore/cache/CacheDir.cc:
##########
@@ -567,7 +548,15 @@ Directory::insert(const CacheKey *key, StripeSM *stripe,
Dir *to_part)
do {
prev = last;
last = next_dir(last, seg);
- } while (last && (++l <= this->buckets * DIR_DEPTH));
+ // Past the longest legitimate chain, so this is provably a cycle. This is
the only walk insert() makes, so it is
+ // where a writer meets one: repair and start over rather than linking
into the cycle.
+ if (++l > this->max_bucket_depth()) {
+ if (this->bucket_loop_fix(b, s)) {
+ goto Lagain;
+ }
+ break;
+ }
Review Comment:
In Directory::insert(), if the chain walk exceeds max_bucket_depth() but
bucket_loop_fix() returns 0, the code breaks out and proceeds to link the new
entry anyway. That can still attach the new entry to an overlong/corrupt chain
(and potentially into a loop if loop detection fails), contradicting the intent
to “repair and start over”. Consider treating an over-max walk as corruption
and restarting after forcibly clearing/fixing the segment, rather than falling
through to link.
--
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]
