fix potential deadlock due to inconsistent locking order

[marten-seemann]

Found by https://github.com/sasha-s/go-deadlock:

POTENTIAL DEADLOCK: Inconsistent locking. saw this ordering in one goroutine:
happened before
stream.go:388 v2.(*Stream).processFlags { s.stateLock.Lock() } <<<<<
stream.go:387 v2.(*Stream).processFlags {  }
stream.go:429 v2.(*Stream).incrSendWindow { // Increase window, unblock a sender }
session.go:647 v2.(*Session).handleStreamMessage { stream.incrSendWindow(hdr, flags) }
session.go:607 v2.(*Session).recvLoop {  }
session.go:562 v2.(*Session).recv { func (s *Session) recv() { }

happened after
session.go:787 v2.(*Session).establishStream { s.streamLock.Lock() } <<<<<
session.go:786 v2.(*Session).establishStream { func (s *Session) establishStream(id uint32) { }
stream.go:395 v2.(*Stream).processFlags { } }
stream.go:429 v2.(*Stream).incrSendWindow { // Increase window, unblock a sender }
session.go:647 v2.(*Session).handleStreamMessage { stream.incrSendWindow(hdr, flags) }
session.go:607 v2.(*Session).recvLoop {  }
session.go:562 v2.(*Session).recv { func (s *Session) recv() { }

in another goroutine: happened before
session.go:266 v2.(*Session).Close { s.streamLock.Lock() } <<<<<
session.go:265 v2.(*Session).Close {  }
session.go:282 v2.(*Session).exitErr { s.Close() }
session.go:565 v2.(*Session).recv { } }

happened after
stream.go:355 v2.(*Stream).forceClose { s.stateLock.Lock() } <<<<<
stream.go:354 v2.(*Stream).forceClose { func (s *Stream) forceClose() { }
session.go:267 v2.(*Session).Close { defer s.streamLock.Unlock() }
session.go:282 v2.(*Session).exitErr { s.Close() }
session.go:565 v2.(*Session).recv { } }

Other goroutines holding locks:
goroutine 18302307 lock 0xc001728e78
session.go:266 v2.(*Session).Close { s.streamLock.Lock() } <<<<<
session.go:265 v2.(*Session).Close {  }
session.go:282 v2.(*Session).exitErr { s.Close() }
session.go:565 v2.(*Session).recv { } }

goroutine 18302307 lock 0xc001728ed8
session.go:250 v2.(*Session).Close { s.shutdownLock.Lock() } <<<<<
session.go:249 v2.(*Session).Close { func (s *Session) Close() error { }
session.go:282 v2.(*Session).exitErr { s.Close() }
session.go:565 v2.(*Session).recv { } }

[vyzo]

Can we have both closeStream and establishStream be true?

Other than that, lgtm.

[marten-seemann]

Can we have both closeStream and establishStream be true?

Yes, I think you can open and close a stream in the same frame. But that's possible today, and I think we should be fine first calling establishStream and then closeStream. This PR shouldn't change the order of execution of these functions, it just makes sure that the stateLock is released before they're executed.

[marten-seemann]

Thinking about it, there might be a race condition if for example closeStream is called concurrently before establishStream.