Kam Kyrala authored Jul 09, 2026 and Stan Hu committed Jul 22, 2026

The package-level s3ClientCache in cache/s3v2 keyed cached AWS clients
by *CacheS3Config pointer identity. RunnerConfig is JSON-deep-copied for
every build (common.NewBuild), so each cache-using job presented a
never-before-seen pointer, built a brand-new S3 client, and stored it in
the sync.Map forever. The invariant broke at the LoadOrStore call site:
pointer keys are only stable within one config load, but the per-job
deep copy defeats that, turning the cache into an unbounded per-job
allocation (~11 KB retained per cache-using job).

Fix: derive a canonical, comparable value key from the effective config.
Pointer-valued options (DualStack, PathStyle) are flattened into
set/value pairs so deep copies compare equal, while any real
configuration difference still yields a distinct entry. Details that
fell out of review:

- Configs without an explicit BucketLocation always bypass the cache:
  region auto-detection falls back to us-east-1 on transient errors,
  and caching would pin the wrong-region client for the process
  lifetime.
- Cached clients store a private deep copy of the config so equal-valued
  callers never alias one caller's mutable struct.
- nil configs are rejected with an error instead of panicking.
- A compile-time guard keeps the key type comparable if CacheS3Config
  grows new fields.

The regression test deep-copies the config through JSON 1,000 times,
mirroring RunnerConfig.DeepCopy, and fails on every iteration without
the fix.

Closes #39595

Kam Kyrala authored Jul 09, 2026 and Stan Hu committed Jul 22, 2026

Collect per-goroutine errors and nil-check clients before the type
assertions, so a failed newS3Client produces a test failure instead of
a panic in the assertion loop.

[19.1] Key S3 client cache by config value, not pointer

See merge request !6982



Merged-by: Taka Nishida <tnishida@gitlab.com>
Approved-by: Taka Nishida <tnishida@gitlab.com>
Co-authored-by: Kam Kyrala <kkyrala@gitlab.com>