tikvclient: fix a bug that double close channels.

[hicqu]

Signed-off-by: qupeng [email protected]

What problem does this PR solve?

There is a bug in tikvclient module about double closing channels, which causes some threads panic when holding a lock. Although these threads can recover, they will be blocked on that lock so they can't work finally.

What is changed and how it works?

This PR fixed the channel double closing problem, and use defer to make the lock logic more clear.

Check List

Tests

• It's hard to add unit tests. We need to cover the case in our tikv client test framework.

Related changes

• Need to cherry-pick to the release branch

[zz-jason]

please add an unit test

[hicqu]

@zz-jason I guess you didn't read the initial comment. Unit tests can't work for these cases because we need some real TiKV services to run tikvclient code. Currently we test tikvclient.go in 2 schrodinger test cases, we can add more cases there instead of here.

[siddontang]

you can use faketikv

seem we also need to care client-go , @disksing

[lysu]

almost lgtm, but have some question..

can we do not call failPendingRequests in sendLoop for errors execept io.EOF?

it seems when sendMsg fail will finish csAttempt https://github.com/grpc/grpc-go/blob/master/stream.go#L670

so it will trigger retry or http2Client#closeStream(will write error to recvBuffer), so it seems recvMsg always got error - -?

..... but maybe do it in both side will more safe

[hicqu]

@lysu I think don't depend gRPC's internal behaviors is better. So let's call failPendingRequests in a locked context.

[codecov]

Codecov Report

Merging #10991 into master will decrease coverage by 0.4708%.
The diff coverage is 72.2222%.

@@               Coverage Diff                @@
##             master     #10991        +/-   ##
================================================
- Coverage   81.5977%   81.1269%   -0.4709%     
================================================
  Files           420        420                
  Lines         90902      89466      -1436     
================================================
- Hits          74174      72581      -1593     
- Misses        11421      11630       +209     
+ Partials       5307       5255        -52

[hicqu]

PTAL @lysu @zz-jason @lonng thanks!

[tiancaiamao]

Please fix CI @hicqu

[tiancaiamao]

/run-all-tests

[tiancaiamao]

/run-all-tests

[tiancaiamao]

/run-all-tests