Skip to content

[8.4] MOD-14461: Fix FT.EXPLAIN missing spec read lock#8792

Merged
raz-mon merged 3 commits into
8.4from
backport-8669-to-8.4
Mar 25, 2026
Merged

[8.4] MOD-14461: Fix FT.EXPLAIN missing spec read lock#8792
raz-mon merged 3 commits into
8.4from
backport-8669-to-8.4

Conversation

@raz-mon
Copy link
Copy Markdown
Collaborator

@raz-mon raz-mon commented Mar 23, 2026

Description

Backport of #8669 to 8.4.

Conflicts Resolved

  • src/aggregate/aggregate_exec.c: The 8.4 branch uses AREQ_Free() while master uses AREQ_DecrRef(). Resolved by keeping AREQ_Free() for the 8.4 branch while applying the locking fix (RedisSearchCtx_UnlockSpec).

Jira

MOD-14624

Release Notes

  • This PR requires release notes

User-impact: Before this fix, the DB may crash if running FT.EXPLAIN while GC updates the index.


Pull Request opened by Augment Code with guidance from the PR author


Note

Medium Risk
Touches spec locking around FT.EXPLAIN, a concurrency-sensitive area; while scoped, mistakes could cause deadlocks or leave locks held.

Overview
Release note: Fixes a potential crash when running FT.EXPLAIN concurrently with index GC/updates by taking a spec read lock while building the execution plan and generating the explain output.

Written by Cursor Bugbot for commit d2d019e. This will update automatically on new commits. Configure here.

* MOD-14461: Fix FT.EXPLAIN missing spec read lock

RS_GetExplainOutput was calling prepareExecutionPlan without first
acquiring a read lock on the index spec, which could lead to race
conditions with the GC modifying spec structures (terms, suffix, docs,
stats) concurrently.

Added RedisSearchCtx_LockSpecRead/UnlockSpec around prepareExecutionPlan
and QAST_DumpExplain, following the same pattern used in other callers.

* Update comment
@jit-ci
Copy link
Copy Markdown

jit-ci Bot commented Mar 23, 2026

🛡️ Jit Security Scan Results

CRITICAL HIGH MEDIUM

✅ No security findings were detected in this PR


Security scan by Jit

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Comment thread src/aggregate/aggregate_exec.c Outdated
@raz-mon raz-mon requested a review from Itzikvaknin March 23, 2026 12:27
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.49%. Comparing base (63b33b6) to head (d2d019e).
⚠️ Report is 3 commits behind head on 8.4.

Additional details and impacted files
@@            Coverage Diff             @@
##              8.4    #8792      +/-   ##
==========================================
- Coverage   85.54%   85.49%   -0.05%     
==========================================
  Files         337      337              
  Lines       53417    53419       +2     
  Branches    11023    11023              
==========================================
- Hits        45695    45670      -25     
- Misses       7579     7606      +27     
  Partials      143      143              
Flag Coverage Δ
flow 84.84% <100.00%> (-0.08%) ⬇️
unit 51.03% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@raz-mon raz-mon enabled auto-merge March 24, 2026 08:33
@sonarqubecloud
Copy link
Copy Markdown

@raz-mon raz-mon added this pull request to the merge queue Mar 25, 2026
Merged via the queue into 8.4 with commit 6049225 Mar 25, 2026
32 of 33 checks passed
@raz-mon raz-mon deleted the backport-8669-to-8.4 branch March 25, 2026 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants