Fix the race condition between goroutine and channel on the same leases to be revoked

[ahrtr]

It's a classic mistake in using the Golang channel + goroutine. When a goroutine accesses data in the closure environment, we should make a copy of the data for the goroutine.

See the example in effective_go#channels

[codecov-commenter]

Codecov Report

Merging #14067 (c35bf87) into main (c3bc411) will decrease coverage by 0.14%.
The diff coverage is 63.63%.

@@            Coverage Diff             @@
##             main   #14067      +/-   ##
==========================================
- Coverage   75.11%   74.96%   -0.15%     
==========================================
  Files         451      451              
  Lines       36791    36793       +2     
==========================================
- Hits        27636    27583      -53     
- Misses       7420     7463      +43     
- Partials     1735     1747      +12     

Flag	Coverage Δ
all	74.96% <63.63%> (-0.15%)	⬇️

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

Impacted Files	Coverage Δ
server/etcdserver/server.go	84.44% <63.63%> (-0.29%)	⬇️
client/pkg/v3/testutil/leak.go	62.83% <0.00%> (-7.08%)	⬇️
client/v3/concurrency/session.go	88.63% <0.00%> (-4.55%)	⬇️
server/etcdserver/txn/util.go	75.47% <0.00%> (-3.78%)	⬇️
server/etcdserver/cluster_util.go	70.35% <0.00%> (-3.17%)	⬇️
server/etcdserver/api/v3rpc/interceptor.go	74.47% <0.00%> (-3.13%)	⬇️
client/v3/concurrency/election.go	79.68% <0.00%> (-2.35%)	⬇️
server/etcdserver/api/rafthttp/peer.go	85.06% <0.00%> (-1.95%)	⬇️
pkg/traceutil/trace.go	96.15% <0.00%> (-1.93%)	⬇️
server/etcdserver/api/v3rpc/key.go	82.19% <0.00%> (-1.37%)	⬇️
... and 11 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c3bc411...c35bf87. Read the comment docs.

[serathius]

Great find! Did you find it yourself or used static analysis. Would be great if we could scan the codebase for more cases like this.

Still I would prefer if we just stop using anonymous functions. Could you maybe refactor this to call a function?

[ahrtr]

I just found it during code review on the lease related code. I am investigating how can we refactor lease. I have a bias for action, so submit a PR to fix it immediately when I see the issue.

Probably some linter tools can discover such issue, and it's in my to do list.

Just moved the code into a separate method, and it looks much better now.

[serathius]

cc @ptabor

[ahrtr]

cc @spzala as well. This is a minor change, and it should be safe.

[ptabor]

Nice finding. Benefits of final variables in Java,

[ptabor]

Neither intelij nor staticcheck is reporting this as problem:
(cd server && .../go/bin/staticcheck ./...) | grep server.go