holepunch: race condition with identifyWait

[sukunrt]

Currently holepunch calls hs.ids.IdentifyWait(conn) inside the netNotifee.Connected call.
But IdentifyWait requires netNotifee.Connected call to identify to be completed.

https://github.com/libp2p/go-libp2p/blob/master/p2p/protocol/holepunch/holepuncher.go#L262

Swarm doesn't guarantee any ordering on notifcations. The notifees are obtained from map keys.

[marten-seemann]

Is that a race condition? The IdentifyWait is called in a Go routine. Wouldn't it just wait until it has received the swarm notification?

[sukunrt]

I think it doesn't. IdentifyWait assumes the Connected call is completed before calling identifyWait.
https://github.com/libp2p/go-libp2p/blob/master/p2p/protocol/identify/id.go#L347

The entry is added here
https://github.com/libp2p/go-libp2p/blob/master/p2p/protocol/identify/id.go#L889

[sukunrt]

the relevant identifyWait line is this.
https://github.com/libp2p/go-libp2p/blob/master/p2p/protocol/identify/id.go#L352

[marten-seemann]

Can you explain how things go wrong when this race condition is triggered?

[sukunrt]

we will skip this block where we try to direct dial before hole punch
https://github.com/libp2p/go-libp2p/blob/master/p2p/protocol/holepunch/holepuncher.go#L110