phongn opened a new pull request, #13380:
URL: https://github.com/apache/trafficserver/pull/13380

   ## Summary
   
   `put(..., copy = true)` is the caller's contract that buffers may not be 
shared between the cache and the caller in either direction. `CacheVC` requests 
it whenever RAM cache compression is configured (`http_copy_hdr`, 
`CacheVC.cc`), because it unmarshals HTTP headers in place in its own buffer 
*after* the put, and again on every RAM-cache hit.
   
   The LRU and S3-FIFO RAM caches ignored the flag and shared buffers both 
ways. With `proxy.config.cache.ram_cache.compress` set and `algorithm = 1` (the 
default) or `2`:
   
   1. put stores the caller's buffer, and CacheVC's subsequent in-place 
unmarshal silently mutates the cached bytes;
   2. the next HTTP RAM hit hands that buffer back and unmarshals it again — 
`HTTPInfo::unmarshal` fails on the already-unmarshalled header (`ink_assert` in 
debug builds; broken HTTP RAM hits in release).
   
   The `http_copy_hdr` gate keys off the compress config only, never the 
algorithm, so this is reachable purely by configuration today. This correctness 
fix stands alone (compression support for LRU is proposed separately) and is a 
backport candidate.
   
   ## Changes
   
   - `RamCacheLRU` and `RamCacheS3FIFO` now honor `copy` the way CLFUS always 
has: copy on put, copy on get, and a `copy = true` put refreshes a resident 
entry that may still be sharing a caller's buffer from before a configuration 
change.
   - `copy = false` behavior is unchanged: zero-copy sharing, pinned by a test.
   - New Catch2 test (`test_RamCacheCopy`) pins the contract for all three 
policies: caller mutation after put, caller mutation of the returned buffer, 
the resident-refresh transition, and copy=false sharing. Before the fix, the 
three copy cases fail for LRU and S3-FIFO; CLFUS passes.
   
   ## Notes for reviewers
   
   - The copied-in buffer is an exact-size xmalloc allocation, so 
`block_size()` recovers the data length and neither entry struct needs a new 
length field.
   - S3-FIFO's ghost-readmit and stale-auxkey paths go through the single 
insert site, so they inherit the copy handling.
   - The test keeps constructed caches reachable rather than destroying them: 
the policies have no destructors (entries are pool-allocated), so destroying a 
cache object would strand its entries under leak checkers.
   


-- 
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