Fix logging race #100
Fix logging race #100
Conversation
ghost
assigned 
limiter.go
Outdated
| log.Debugf("[limiter] executing dial (dialfunc); peer: %s; addr: %s; FD consuming: %d; waiting: %d", | ||
| j.peer, j.addr, dl.fdConsuming, len(dl.waitingOnFd)) | ||
| dl.lk.Unlock() |
There was a problem hiding this comment.
what's the race here? log should be thread-safe.
|
I’m ok with this data race. It’s not worthwhile to sync for a log that does not require consistency and that will not be printed in 99% of cases (debug). Even if we sync’ed, it would not have a visible effect. Any way to quieten the race checker? Reopen if you think this is a mistake. |
|
Unfortunately, there is no way to quiet the racechecker. Maybe we should just remove the log, the dial will be logged esewhere. |
|
@Kubuxu I'm not one to remove logs, but it's justified in this case because this statement is actually superfluous. The original one is here: https://github.com/libp2p/go-libp2p-swarm/blob/master/limiter.go#L162, and we trust Go enough to launch the goroutine. So this statement adds no value. |
Fix race caused by log statement. Merge #99 to see its effect.