Skip to content

Add NULL check in updateSSLPendingFlag#3641

Merged
zuiderkwast merged 1 commit into
valkey-io:unstablefrom
zuiderkwast:tls-pending-null-check
May 6, 2026
Merged

Add NULL check in updateSSLPendingFlag#3641
zuiderkwast merged 1 commit into
valkey-io:unstablefrom
zuiderkwast:tls-pending-null-check

Conversation

@zuiderkwast
Copy link
Copy Markdown
Contributor

Fixes #3607

Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
@zuiderkwast zuiderkwast moved this to In Progress / Needs Review in Valkey 7.2 May 6, 2026
@zuiderkwast zuiderkwast moved this to Todo in Valkey 8.0 May 6, 2026
@zuiderkwast zuiderkwast moved this to In Progress / Needs Review in Valkey 8.1 May 6, 2026
@zuiderkwast zuiderkwast moved this to In Progress / Needs Review in Valkey 9.0 May 6, 2026
@zuiderkwast zuiderkwast moved this to Needs Review in Valkey 9.1 May 6, 2026
@zuiderkwast zuiderkwast requested a review from ranshid May 6, 2026 16:28
@codecov
Copy link
Copy Markdown

codecov Bot commented May 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.69%. Comparing base (96763ad) to head (92fba6e).
⚠️ Report is 1 commits behind head on unstable.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #3641      +/-   ##
============================================
+ Coverage     76.67%   76.69%   +0.01%     
============================================
  Files           162      162              
  Lines         80623    80623              
============================================
+ Hits          61816    61830      +14     
+ Misses        18807    18793      -14     
Files with missing lines Coverage Δ
src/tls.c 17.64% <ø> (ø)

... and 21 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

@murphyjacob4 murphyjacob4 left a comment

Choose a reason for hiding this comment

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

This guard LGTM and it's good to be defensive here.

But conn->ssl == NULL implies that shutdown or close was previously called. Do we have a bug in the state machine? Ideally after shutdown or close we would move the connection state to reflect that, and fail fast in connTLSRead with the check:

if (conn->c.state != CONN_STATE_CONNECTED) return -1;

I guess we don't move the state machine on shutdown or close in the existing code?

@zuiderkwast
Copy link
Copy Markdown
Contributor Author

zuiderkwast commented May 6, 2026

Do we have a bug in the state machine?

I don't know how it happened, but I know from #3607 that it happened and that this fix prevents the crash.

If we can't reproduce it, we can only try to follow all call sites to see how this can happen.

Let's backport this small fix and then we can improve the state machine in future versions?

@zuiderkwast zuiderkwast merged commit 2f16107 into valkey-io:unstable May 6, 2026
60 of 61 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress / Needs Review to To be backported in Valkey 8.1 May 6, 2026
@github-project-automation github-project-automation Bot moved this from Todo to To be backported in Valkey 8.0 May 6, 2026
@github-project-automation github-project-automation Bot moved this from Needs Review to To be backported in Valkey 9.1 May 6, 2026
@github-project-automation github-project-automation Bot moved this from In Progress / Needs Review to To be backported in Valkey 7.2 May 6, 2026
@github-project-automation github-project-automation Bot moved this from In Progress / Needs Review to To be backported in Valkey 9.0 May 6, 2026
@zuiderkwast zuiderkwast deleted the tls-pending-null-check branch May 6, 2026 17:06
sarthakaggarwal97 pushed a commit to sarthakaggarwal97/valkey that referenced this pull request May 7, 2026
Fixes valkey-io#3607

Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
sarthakaggarwal97 pushed a commit to sarthakaggarwal97/valkey that referenced this pull request May 7, 2026
Fixes valkey-io#3607

Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
sarthakaggarwal97 pushed a commit to sarthakaggarwal97/valkey that referenced this pull request May 7, 2026
Fixes valkey-io#3607

Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
sarthakaggarwal97 pushed a commit to sarthakaggarwal97/valkey that referenced this pull request May 10, 2026
Fixes valkey-io#3607

Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
lucasyonge pushed a commit that referenced this pull request May 11, 2026
Fixes #3607

Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
lucasyonge pushed a commit that referenced this pull request May 12, 2026
Fixes #3607

Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
@sarthakaggarwal97 sarthakaggarwal97 moved this from To be backported to Done in Valkey 9.1 May 16, 2026
valkeyrie-ops Bot pushed a commit that referenced this pull request May 18, 2026
Fixes #3607

Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
valkeyrie-ops Bot pushed a commit that referenced this pull request May 19, 2026
Fixes #3607

Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
valkeyrie-ops Bot pushed a commit that referenced this pull request May 19, 2026
Fixes #3607

Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
valkeyrie-ops Bot pushed a commit that referenced this pull request May 21, 2026
Fixes #3607

Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
valkeyrie-ops Bot pushed a commit that referenced this pull request May 29, 2026
Fixes #3607

Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
zuiderkwast added a commit that referenced this pull request Jun 2, 2026
Fixes #3607

Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
@zuiderkwast zuiderkwast added the release-notes This issue should get a line item in the release notes label Jun 2, 2026
@zuiderkwast zuiderkwast changed the title Add null check in updateSSLPendingFlag Add NULL check in updateSSLPendingFlag Jun 2, 2026
@zuiderkwast zuiderkwast moved this from To be backported to 8.1.8 (WIP) in Valkey 8.1 Jun 2, 2026
zuiderkwast added a commit that referenced this pull request Jun 2, 2026
Fixes #3607

Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes This issue should get a line item in the release notes

Projects

Status: To be backported
Status: To be backported
Status: 8.1.8
Status: To be backported
Status: Done

Development

Successfully merging this pull request may close these issues.

[CRASH] Valkey 8.1.6 crashed during sync with 8.0.7 node

3 participants