Skip to content

fix: potential race conditions

Some of the changes of the first ("err") commit do not appear to have a potential race condition, so there it is purely a refactor, while in others (e.g. in broker.go and in proxy/lib/snowflake.go) do use the same variable from multiple threads / functions.

As for the second commit: Although it does not look like that there are situations where it is critical to use a mutex, because it's only used when performing rendezvous with a proxy, which doesn't happen too frequently, let's still do it just to be sure.

Update: I pushed another commit fix: `periodicProxyStats.connectionCount` race. I discovered it after running a proxy with the -race flag and getting this error:

log
==================
WARNING: DATA RACE
Read at ___ by goroutine ___:
  gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/v2/proxy/lib.(*periodicProxyStats).OnNewSnowflakeEvent()
      .../snowflake/proxy/lib/pt_event_logger.go:62 +0x84
  gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/v2/common/event.(*eventBus).OnNewSnowflakeEvent()
      .../snowflake/common/event/bus.go:18 +0x13c
  gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/v2/proxy/lib.(*SnowflakeProxy).makePeerConnectionFromOffer.func1.3()
      .../snowflake/proxy/lib/snowflake.go:496 +0x4c1

Previous write at ___ by goroutine ___:
  gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/v2/proxy/lib.(*periodicProxyStats).logTick()
      .../snowflake/proxy/lib/pt_event_logger.go:75 +0x18e
  gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/v2/proxy/lib.(*periodicProxyStats).logTick-fm()
      <autogenerated>:1 +0x33
  gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/v2/common/task.(*Periodic).checkedExecute()
      .../snowflake/common/task/periodic.go:59 +0x75
  gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/v2/common/task.(*Periodic).checkedExecute.func1()
      .../snowflake/common/task/periodic.go:79 +0x2e

Goroutine ___ (running) created at:
  github.com/pion/webrtc/v4.(*DataChannel).onClose()
      .../go/pkg/mod/github.com/pion/webrtc/v4@v4.0.8/datachannel.go:290 +0x64
  github.com/pion/webrtc/v4.(*DataChannel).readLoop()
      .../go/pkg/mod/github.com/pion/webrtc/v4@v4.0.8/datachannel.go:403 +0x211
  github.com/pion/webrtc/v4.(*DataChannel).handleOpen.gowrap2()
      .../go/pkg/mod/github.com/pion/webrtc/v4@v4.0.8/datachannel.go:362 +0x33

Goroutine ___ (finished) created at:
  time.goFunc()
      /usr/local/go/src/time/sleep.go:215 +0x44
==================
Edited by WofWca

Merge request reports

Loading