server: make clientConn() thread-safe
#49073
Conversation
ti-chi-bot
bot
added
do-not-merge/needs-triage-completed
release-note-none
bb7133
changed the title
clientConn() thread-safe
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #49073 +/- ##
================================================
+ Coverage 71.0454% 72.0865% +1.0411%
================================================
Files 1368 1405 +37
Lines 402993 413291 +10298
================================================
+ Hits 286308 297927 +11619
+ Misses 96744 96554 -190
+ Partials 19941 18810 -1131
Flags with carried forward coverage won't be shown. Click here to find out more.
|
ti-chi-bot
bot
added
needs-cherry-pick-release-6.1
ti-chi-bot
bot
added
approved
needs-1-more-lgtm
| @@ -349,9 +350,13 @@ func (cc *clientConn) Close() error { | |||
| return closeConn(cc, connections) | |||
| } | |||
|
|
|||
| // closeConn should be idempotent. | |||
| // closeConn is idempotent and thread-safe. | |||
There was a problem hiding this comment.
Do we need to ensure that concurrent invocations of closeConn are blocked until the first closeConn is finished? like sync.Once. I'm not sure if the second caller see closeConn is finished it will access other objects and will race with the running closeConn
There was a problem hiding this comment.
Good point! I'll update the code.
There was a problem hiding this comment.
PTAL @lance6716 , and I found that my previous implementation is explicitly noted as 'the incorrect one' in the comment of sync.Once
bb7133
force-pushed
the
bb7133/safe_close
branch
from
cb290ba
to
ccdc31d
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lance6716, tiancaiamao The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ti-chi-bot
bot
added
lgtm
and removed
needs-1-more-lgtm
[LGTM Timeline notifier]Timeline:
|
|
In response to a cherrypick label: new pull request created to branch |
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
|
In response to a cherrypick label: new pull request created to branch |
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
|
In response to a cherrypick label: new pull request created to branch |
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
|
This PR also fix #42761 👍 |
jackysp
added
the
needs-cherry-pick-release-7.5
|
In response to a cherrypick label: new pull request created to branch |
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
What problem does this PR solve?
Issue Number: ref #48224, #36793 and #32110
Problem Summary:
What changed and how does it work?
Please check #36793 (comment) for more details and context:
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.