Skip to content

fix(pipe): avoid nil cache panic with disabled static-TTL multi-cache - #1024

Merged
rueian merged 2 commits into
redis:mainfrom
kurochan:fix-nil-cache-panic
Aug 19, 2026
Merged

rueian merged 2 commits into
redis:mainfrom
kurochan:fix-nil-cache-panic

Conversation

@kurochan

@kurochan kurochan commented Aug 18, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fix a nil pointer panic when DoMultiCache is used with multiple
ToStaticTTL() commands while client-side caching is disabled.

Root cause

With DisableCache: true, the pipe does not initialize p.cache.

The DoMultiCache fallback converted Cacheable commands to Completed
commands but preserved the internal static-TTL flag. In the background read
loop, the second and subsequent replies then entered the static-TTL path and
called p.cache.Update or p.cache.Cancel, causing a nil pointer panic.

Changes

  • Clear static-TTL metadata before forwarding disabled-cache requests to
    DoMulti.
  • Guard the background static-TTL path when no cache store exists.
  • Add regression tests for successful replies and Redis errors with multiple
    static-TTL commands and a cancelable context.

Compatibility

  • DisableCache remains equivalent to Do/DoMulti.
  • Client-side caching behavior is unchanged when enabled.
  • Redis errors are returned to callers unchanged.

Note

Low Risk
Narrow fix for DisableCache + static-TTL edge case; enabled client-side caching paths are unchanged aside from the same clear-on-fallback in DoCache.

Overview
Fixes a nil pointer panic when client-side caching is off (DisableCache: true) but callers still use DoCache / DoMultiCache with ToStaticTTL() commands.

With no cache store, those APIs already fall back to plain Do / DoMulti, but the static-TTL flag stayed on the wire commands. The background read loop then treated later replies as cacheable and called p.cache.Update / Cancel on a nil store—especially on the second+ command in a DoMultiCache batch.

Change: strip static-TTL metadata via cmds.ClearStaticTTL before forwarding in both disabled-cache paths so behavior matches untagged Do/DoMulti. Regression tests cover multi-command success, mixed Redis errors, and single DoCache.

Reviewed by Cursor Bugbot for commit 4245f7a. Bugbot is set up for automated code reviews on this repo. Configure here.

@kurochan
kurochan force-pushed the fix-nil-cache-panic branch from 235762d to 1f77177 Compare August 18, 2026 04:43
Comment thread pipe.go
for i, ct := range multi {
commands[i] = Completed(ct.Cmd)
cmd := ct.Cmd
cmds.ClearStaticTTL(&cmd)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to ClearStaticTTL at DoCache as well?

@rueian
rueian merged commit 12cec18 into redis:main Aug 19, 2026
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants