fix(core): fix bad connection state after TLS session initialization failure#6529
fix(core): fix bad connection state after TLS session initialization failure#6529bluestreak01 merged 1 commit intomasterfrom
Conversation
…failure When TLS session initialization failed, then the connection context was closed twice, and it could be also pooled twice. This could lead to issues in later connections. Joint work with @RaphDal
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
hello @coderabbitai, my favourite robot, please review this fine art! :) |
🧩 Analysis chain🏁 Script executed: Length of output: 1380 🏁 Script executed: Length of output: 1537 🏁 Script executed: Length of output: 2990 🏁 Script executed: Length of output: 1887 @jerrinot, Hello! Thank you for the kind words! 🐰 I'd be delighted to review this fine piece of work! Review SummaryThis is an excellent fix for a subtle but serious bug! The change is minimal, surgical, and addresses the root cause perfectly. AnalysisThe ProblemWhen TLS session initialization failed, the exception handler was calling both:
This caused a double-cleanup scenario where:
The SolutionBy removing the immediate Why This WorksLooking at the code flow:
The manual call to Verdict✅ LGTM! This is a textbook example of how to fix a resource management bug - identify the double-cleanup, remove the redundant call, and let the established cleanup flow handle it consistently. The one-line change speaks to the quality of the analysis that went into finding this issue. Great work! 🎉 🐰✨ ✅ Actions performedComments resolved. |
[PR Coverage check]😍 pass : 0 / 0 (0%) |
When TLS session initialization failed, then the connection context was closed twice, and it could be also pooled twice. This could lead to issues in later connections.
Joint work with @RaphDal
Note for reviewers:
disconnect()enqueues the context in thedisconnectQueue. The queue is later processed, and the connection is closed.