Skip to content

Fix TSAN warning in hashTypeLength#15382

Merged
tezc merged 1 commit into
redis:unstablefrom
tezc:fix-tsan
Jun 29, 2026
Merged

Fix TSAN warning in hashTypeLength#15382
tezc merged 1 commit into
redis:unstablefrom
tezc:fix-tsan

Conversation

@tezc

@tezc tezc commented Jun 28, 2026

Copy link
Copy Markdown
Collaborator

TSan complains because server.allow_access_expired is accessed from a BIO thread: populateDeltaHistograms() reaches hashTypeLength() via getObjectLength() with subtractExpiredFields==0.

Fix it by reading that variable only when necessary, i.e. only when subtractExpiredFields is set.

*** [err]: Sanitizer error: ==================
WARNING: ThreadSanitizer: data race (pid=77380)
  Write of size 4 at 0x55e70ee77980 by main thread (mutexes: write M0):
    #0 moduleNotifyKeyUnlink /home/runner/work/redis/redis/src/module.c:13193 (redis-server+0x33246d) (BuildId: 56f4df0f306eff979d7ac5c7c665610cf8c07edb)
    #1 dbSetValue /home/runner/work/redis/redis/src/db.c:623 (redis-server+0x158aa4) (BuildId: 56f4df0f306eff979d7ac5c7c665610cf8c07edb)
    #2 setKeyByLink /home/runner/work/redis/redis/src/db.c:777 (redis-server+0x159406) (BuildId: 56f4df0f306eff979d7ac5c7c665610cf8c07edb)
    #3 setGenericCommand /home/runner/work/redis/redis/src/t_string.c:181 (redis-server+0x1a1ce2) (BuildId: 56f4df0f306eff979d7ac5c7c665610cf8c07edb)
    #4 setCommand /home/runner/work/redis/redis/src/t_string.c:443 (redis-server+0x1a22d8) (BuildId: 56f4df0f306eff979d7ac5c7c665610cf8c07edb)
    #5 call /home/runner/work/redis/redis/src/server.c:4035 (redis-server+0xe719f) (BuildId: 56f4df0f306eff979d7ac5c7c665610cf8c07edb)
    #6 processCommand /home/runner/work/redis/redis/src/server.c:4808 (redis-server+0xea119) (BuildId: 56f4df0f306eff979d7ac5c7c665610cf8c07edb)
    #7 processCommandAndResetClient /home/runner/work/redis/redis/src/networking.c:3497 (redis-server+0x13e076) (BuildId: 56f4df0f306eff979d7ac5c7c665610cf8c07edb)
    #8 processInputBuffer /home/runner/work/redis/redis/src/networking.c:3789 (redis-server+0x13e076)
    #9 processPendingCommandAndInputBuffer /home/runner/work/redis/redis/src/networking.c:3541 (redis-server+0x13fadf) (BuildId: 56f4df0f306eff979d7ac5c7c665610cf8c07edb)
    #10 processClientsFromIOThread /home/runner/work/redis/redis/src/iothread.c:619 (redis-server+0xc1cfa) (BuildId: 56f4df0f306eff979d7ac5c7c665610cf8c07edb)
    #11 handleClientsFromIOThread /home/runner/work/redis/redis/src/iothread.c:680 (redis-server+0xc1e59) (BuildId: 56f4df0f306eff979d7ac5c7c665610cf8c07edb)
    #12 aeProcessEvents /home/runner/work/redis/redis/src/ae.c:435 (redis-server+0xa861d) (BuildId: 56f4df0f306eff979d7ac5c7c665610cf8c07edb)
    #13 aeProcessEvents /home/runner/work/redis/redis/src/ae.c:360 (redis-server+0xa861d)
    #14 aeMain /home/runner/work/redis/redis/src/ae.c:495 (redis-server+0xa861d)
    #15 main /home/runner/work/redis/redis/src/server.c:8327 (redis-server+0x8b4c4) (BuildId: 56f4df0f306eff979d7ac5c7c665610cf8c07edb)

  Previous read of size 4 at 0x55e70ee77980 by thread T3:
    #0 hashTypeLength /home/runner/work/redis/redis/src/t_hash.c:2299 (redis-server+0x1eb6b5) (BuildId: 56f4df0f306eff979d7ac5c7c665610cf8c07edb)
    #1 getObjectLength /home/runner/work/redis/redis/src/object.c:1009 (redis-server+0x151d69) (BuildId: 56f4df0f306eff979d7ac5c7c665610cf8c07edb)
    #2 populateDeltaHistograms /home/runner/work/redis/redis/src/lazyfree.c:33 (redis-server+0x2fd505) (BuildId: 56f4df0f306eff979d7ac5c7c665610cf8c07edb)
    #3 kvsLazyfreeFree /home/runner/work/redis/redis/src/lazyfree.c:63 (redis-server+0x2fd505)
    #4 bioProcessBackgroundJobs /home/runner/work/redis/redis/src/bio.c:340 (redis-server+0x2b07cb) (BuildId: 56f4df0f306eff979d7ac5c7c665610cf8c07edb)
    ```

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Low Risk**
> Single guarded read reorder in hash length counting; intended behavior is preserved for subtractExpiredFields=1 callers.
> 
> **Overview**
> Fixes a ThreadSanitizer **data race** on `server.allow_access_expired` when background I/O computes hash lengths for keysizes histograms.
> 
> **`hashTypeLength`** now reads `server.allow_access_expired` only when `subtractExpiredFields` is true. Callers such as **`populateDeltaHistograms()`** on a BIO thread use `subtractExpiredFields==0`, so they no longer touch that flag while the main thread updates it during normal command processing. The “don’t subtract expired fields when access is allowed” behavior is unchanged for paths that actually request subtraction.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 042fdc6bbc74f7c33b3678ee4bd994f04a138cba. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

TSan complains because server.allow_access_expired is accessed from a BIO thread: populateDeltaHistograms() reaches hashTypeLength via getObjectLength() with subtractExpiredFields==0. Fix it by reading that variable only when necessary, i.e. only when subtractExpiredFields is set.
@tezc tezc requested a review from moticless June 28, 2026 12:07
@fcostaoliveira

Copy link
Copy Markdown
Collaborator

This change touches performance-sensitive code paths. Adding the action:run-benchmark label will trigger the CE Performance suite so we can see the impact before merge.

@sundb sundb requested a review from ShooterIT June 29, 2026 01:38
@tezc tezc merged commit fe5d218 into redis:unstable Jun 29, 2026
18 checks passed
@tezc tezc deleted the fix-tsan branch June 29, 2026 05:56
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.

4 participants