search

filecoin-project / go-data-transfer

Data Transfer Shared Component for go-filecoin & go-lotus
Other
39 stars 17 forks source link

Graphsync data race on cleanup #206

Closed tchardin closed 3 years ago

tchardin commented 3 years ago

@dirkmc when running from commit e69af103f5f9a249883ab8c9e46cae63b581a58b a basic data transfer test with -race flag returns: (Doesn't happen all the time but running with higher GOMAXPROCS helps triggering the warning)

WARNING: DATA RACE
Read at 0x00c0002120f0 by goroutine 250:
  runtime.mapiterinit()
      /usr/local/Cellar/go/1.16.3/libexec/src/runtime/map.go:802 +0x0
  github.com/filecoin-project/go-data-transfer/transport/graphsync.gsKeyToChannelIDMap.forEach()
      /Users/tchardin/go-data-transfer/transport/graphsync/graphsync.go:1122 +0x154
  github.com/filecoin-project/go-data-transfer/transport/graphsync.(*Transport).gsNetworkReceiveErrorListener()
      /Users/tchardin/go-data-transfer/transport/graphsync/graphsync.go:710 +0x115
  github.com/filecoin-project/go-data-transfer/transport/graphsync.(*Transport).gsNetworkReceiveErrorListener-fm()
      /Users/tchardin/go-data-transfer/transport/graphsync/graphsync.go:707 +0x72
  github.com/ipfs/go-graphsync/listeners.receiverNetworkErrorDispatcher()
      /Users/tchardin/go/pkg/mod/github.com/ipfs/go-graphsync@v0.7.0/listeners/listeners.go:154 +0xa9
  github.com/hannahhoward/go-pubsub.(*PubSub).Publish()
      /Users/tchardin/go/pkg/mod/github.com/hannahhoward/go-pubsub@v0.0.0-20200423002714-8d62886cc36e/pubsub.go:76 +0x184
  github.com/ipfs/go-graphsync/listeners.(*NetworkReceiverErrorListeners).NotifyNetworkErrorListeners()
      /Users/tchardin/go/pkg/mod/github.com/ipfs/go-graphsync@v0.7.0/listeners/listeners.go:170 +0x212
  github.com/ipfs/go-graphsync/impl.(*graphSyncReceiver).ReceiveError()
      /Users/tchardin/go/pkg/mod/github.com/ipfs/go-graphsync@v0.7.0/impl/graphsync.go:310 +0x126

Previous write at 0x00c0002120f0 by goroutine 42:
  runtime.mapdelete()
      /usr/local/Cellar/go/1.16.3/libexec/src/runtime/map.go:685 +0x0
  github.com/filecoin-project/go-data-transfer/transport/graphsync.gsKeyToChannelIDMap.deleteRefs()
      /Users/tchardin/go-data-transfer/transport/graphsync/graphsync.go:1134 +0x344
=== RUN
  github.com/filecoin-project/go-data-transfer/transport/graphsync.(*dtChannel).cleanup()
      /Users/tchardin/go-data-transfer/transport/graphsync/graphsync.go:1060 +0x26e
  github.com/filecoin-project/go-data-transfer/transport/graphsync.(*Transport).CleanupChannel()
      /Users/tchardin/go-data-transfer/transport/graphsync/graphsync.go:262 +0x12e
  github.com/filecoin-project/go-data-transfer/impl.(*channelEnvironment).CleanupChannel()
      /Users/tchardin/go-data-transfer/impl/environment.go:26 +0xb3
  github.com/filecoin-project/go-data-transfer/channels.cleanupConnection()
      /Users/tchardin/go-data-transfer/channels/channels_fsm.go:218 +0x179
  runtime.call512()
      /usr/local/Cellar/go/1.16.3/libexec/src/runtime/asm_amd64.s:555 +0x5b
  reflect.Value.Call()
      /usr/local/Cellar/go/1.16.3/libexec/src/reflect/value.go:337 +0xd8
  github.com/filecoin-project/go-statemachine/fsm.fsmHandler.handler.func1()
      /Users/tchardin/go/pkg/mod/github.com/filecoin-project/go-statemachine@v0.0.0-20200925024713-05bd7c71fbfe/fsm/fsm.go:170 +0x484
  reflect.callReflect()
      /usr/local/Cellar/go/1.16.3/libexec/src/reflect/value.go:565 +0x5cd
  reflect.makeFuncStub()
      /usr/local/Cellar/go/1.16.3/libexec/src/reflect/asm_amd64.s:22 +0x41
  reflect.Value.Call()
      /usr/local/Cellar/go/1.16.3/libexec/src/reflect/value.go:337 +0xd8
  github.com/filecoin-project/go-statemachine.(*StateMachine).run.func3()
      /Users/tchardin/go/pkg/mod/github.com/filecoin-project/go-statemachine@v0.0.0-20200925024713-05bd7c71fbfe/machine.go:102 +0x279

Goroutine 250 (running) created at:
  github.com/ipfs/go-graphsync/network.(*libp2pGraphSyncNetwork).handleNewStream()
      /Users/tchardin/go/pkg/mod/github.com/ipfs/go-graphsync@v0.7.0/network/libp2p_impl.go:142 +0x65b
  github.com/ipfs/go-graphsync/network.(*libp2pGraphSyncNetwork).handleNewStream-fm()
      /Users/tchardin/go/pkg/mod/github.com/ipfs/go-graphsync@v0.7.0/network/libp2p_impl.go:126 +0x5e
  github.com/libp2p/go-libp2p/p2p/host/basic.(*BasicHost).SetStreamHandler.func1()
      /Users/tchardin/go/pkg/mod/github.com/libp2p/go-libp2p@v0.13.0/p2p/host/basic/basic_host.go:566 +0xb9

Goroutine 42 (running) created at:
  github.com/filecoin-project/go-statemachine.(*StateMachine).run()
      /Users/tchardin/go/pkg/mod/github.com/filecoin-project/go-statemachine@v0.0.0-20200925024713-05bd7c71fbfe/machine.go:100 +0x796

Thought I'd flag it, let me know if you need more help reproducing.

tchardin commented 3 years ago

Doesn't seem to happen on v1.5

dirkmc commented 3 years ago

Thanks @tchardin

I believe this is fixed by https://github.com/filecoin-project/go-data-transfer/pull/208

Please reopen this issue if you see it again.