deadlock: acquire multiple locks in the same order in clientconn

[skinowski]

What version of gRPC are you using?

v1.24.0

What version of Go are you using (go version)?

go version go1.13 linux/amd64

What operating system (Linux, Windows, …) and version?

arch linux - kernel 5.3.1

What did you do?

We are using experimental ResetConnectBackoff() and we kick off gRPC requests
using FailFast(false) for a pool of connections. In some connections, ResetConnectBackoff() blocks exhibiting deadlock behavior. Quick look at the gRPC code reveals multiple mutex locking
order that is not recommended.

For example:

func (cc *ClientConn) ResetConnectBackoff() {
	cc.mu.Lock()
	defer cc.mu.Unlock()
	for ac := range cc.conns {
		ac.resetConnectBackoff()
                // where ac.resetConnectBackoff() calls ac.mu.Lock() for each ac.
	}
}

func (ac *addrConn) resetTransport() {
	for i := 0; ; i++ {

		ac.mu.Lock()

               // deleted code for clarity

		ac.updateConnectivityState(connectivity.Connecting)
                // ac.updateConnectivityState() calls, ac.cc.handleSubConnStateChange(), which
                // calls ac.cc.mu.Lock()

		ac.mu.Unlock()

       // deleted code for clarity

The pattern above exhibits a text book deadlock pattern as follows:

// ResetConnectBackoff() behavior:
m1.lock()
m2.lock()
m2.unlock()
m1.unlock()

// resetTransport() behavior:
m2.lock()
m1.lock()
m1.unlock()
m2.unlock()

What did you expect to see?

multiple locks must be acquired in the same order to avoid deadlock.

What did you see instead?

deadlock

[dfawley]

Thanks for filing this issue! It should be fixed by #3051.