Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: race condition in peermgt initialization reported by race detector #646

Merged
merged 1 commit into from
Aug 16, 2023
Merged

fix: race condition in peermgt initialization reported by race detector #646

merged 1 commit into from
Aug 16, 2023

Conversation

chaitanyaprem
Copy link
Collaborator

@chaitanyaprem chaitanyaprem commented Aug 16, 2023 •
edited
Loading

Description

Ran tests with race detector enabled which reported 2 race conditions (given below).
Fix done for the same by ordering initialization.

Changes

  • Set peermanager host before starting peer connector

Tests

Executed all unit tests with and without race condition.

Race conditions Reported


==================
WARNING: DATA RACE
Read at 0x00c001db9fb0 by goroutine 2437:
  github.com/waku-org/go-waku/waku/v2/peermanager.(*PeerManager).GroupPeersByDirection()
      /Users/prem/Code/go-waku/waku/v2/peermanager/peer_manager.go:106 +0x3c
  github.com/waku-org/go-waku/waku/v2/peermanager.(*PeerConnectionStrategy).shouldDialPeers()
      /Users/prem/Code/go-waku/waku/v2/peermanager/peer_connector.go:188 +0x228
  github.com/waku-org/go-waku/waku/v2/peermanager.(*PeerConnectionStrategy).Start.func1()
      /Users/prem/Code/go-waku/waku/v2/peermanager/peer_connector.go:141 +0x4c

Previous write at 0x00c001db9fb0 by goroutine 2233:
  github.com/waku-org/go-waku/waku/v2/peermanager.(*PeerManager).SetHost()
      /Users/prem/Code/go-waku/waku/v2/peermanager/peer_manager.go:75 +0xebc
  github.com/waku-org/go-waku/waku/v2/node.(*WakuNode).Start()
      /Users/prem/Code/go-waku/waku/v2/node/wakunode2.go:401 +0xea0
  github.com/waku-org/go-waku/waku/v2/node.Test500()
      /Users/prem/Code/go-waku/waku/v2/node/wakunode2_test.go:136 +0x3d0
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1576 +0x188
  testing.(*T).Run.func1()
      /usr/local/go/src/testing/testing.go:1629 +0x40

Goroutine 2437 (running) created at:
  github.com/waku-org/go-waku/waku/v2/peermanager.(*PeerConnectionStrategy).Start()
      /Users/prem/Code/go-waku/waku/v2/peermanager/peer_connector.go:141 +0x25c
  github.com/waku-org/go-waku/waku/v2/node.(*WakuNode).Start()
      /Users/prem/Code/go-waku/waku/v2/node/wakunode2.go:388 +0xe0c
  github.com/waku-org/go-waku/waku/v2/node.Test500()
      /Users/prem/Code/go-waku/waku/v2/node/wakunode2_test.go:136 +0x3d0
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1576 +0x188
  testing.(*T).Run.func1()
      /usr/local/go/src/testing/testing.go:1629 +0x40

Goroutine 2233 (running) created at:
  testing.(*T).Run()
      /usr/local/go/src/testing/testing.go:1629 +0x5e4
  testing.runTests.func1()
      /usr/local/go/src/testing/testing.go:2036 +0x80
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1576 +0x188
  testing.runTests()
      /usr/local/go/src/testing/testing.go:2034 +0x700
  testing.(*M).Run()
      /usr/local/go/src/testing/testing.go:1906 +0x950
  main.main()
      _testmain.go:61 +0x300
==================


==================
WARNING: DATA RACE
Read at 0x00c003412570 by goroutine 3296:
  github.com/waku-org/go-waku/waku/v2/peermanager.(*PeerManager).GroupPeersByDirection()
      /Users/prem/Code/go-waku/waku/v2/peermanager/peer_manager.go:106 +0x3c
  github.com/waku-org/go-waku/waku/v2/peermanager.(*PeerConnectionStrategy).shouldDialPeers()
      /Users/prem/Code/go-waku/waku/v2/peermanager/peer_connector.go:188 +0x228
  github.com/waku-org/go-waku/waku/v2/peermanager.(*PeerConnectionStrategy).Start.func1()
      /Users/prem/Code/go-waku/waku/v2/peermanager/peer_connector.go:141 +0x4c

Previous write at 0x00c003412570 by goroutine 3064:
  github.com/waku-org/go-waku/waku/v2/peermanager.(*PeerManager).SetHost()
      /Users/prem/Code/go-waku/waku/v2/peermanager/peer_manager.go:75 +0xebc
  github.com/waku-org/go-waku/waku/v2/node.(*WakuNode).Start()
      /Users/prem/Code/go-waku/waku/v2/node/wakunode2.go:401 +0xea0
  github.com/waku-org/go-waku/waku/v2/node.TestDecoupledStoreFromRelay()
      /Users/prem/Code/go-waku/waku/v2/node/wakunode2_test.go:247 +0x6b4
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1576 +0x188
  testing.(*T).Run.func1()
      /usr/local/go/src/testing/testing.go:1629 +0x40

Goroutine 3296 (running) created at:
  github.com/waku-org/go-waku/waku/v2/peermanager.(*PeerConnectionStrategy).Start()
      /Users/prem/Code/go-waku/waku/v2/peermanager/peer_connector.go:141 +0x25c
  github.com/waku-org/go-waku/waku/v2/node.(*WakuNode).Start()
      /Users/prem/Code/go-waku/waku/v2/node/wakunode2.go:388 +0xe0c
  github.com/waku-org/go-waku/waku/v2/node.TestDecoupledStoreFromRelay()
      /Users/prem/Code/go-waku/waku/v2/node/wakunode2_test.go:247 +0x6b4
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1576 +0x188
  testing.(*T).Run.func1()
      /usr/local/go/src/testing/testing.go:1629 +0x40

Goroutine 3064 (running) created at:
  testing.(*T).Run()
      /usr/local/go/src/testing/testing.go:1629 +0x5e4
  testing.runTests.func1()
      /usr/local/go/src/testing/testing.go:2036 +0x80
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1576 +0x188
  testing.runTests()
      /usr/local/go/src/testing/testing.go:2034 +0x700
  testing.(*M).Run()
      /usr/local/go/src/testing/testing.go:1906 +0x950
  main.main()
      _testmain.go:61 +0x300
==================

@status-im-auto
Copy link

status-im-auto commented Aug 16, 2023 •
edited
Loading

Jenkins Builds

❔ Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 4143d28 #1 2023-08-16 06:46:49 ~1 min linux 📦deb
✔️ 4143d28 #1 2023-08-16 06:47:42 ~2 min nix-flake 📄log
✔️ 4143d28 #1 2023-08-16 06:48:10 ~2 min tests 📄log
✔️ 4143d28 #1 2023-08-16 06:48:11 ~2 min tests 📄log
✔️ 4143d28 #1 2023-08-16 06:49:05 ~3 min android 📦tgz
✔️ 4143d28 #1 2023-08-16 06:49:26 ~3 min ios 📦tgz

@chaitanyaprem chaitanyaprem merged commit f263be4 into master Aug 16, 2023
3 checks passed
@chaitanyaprem chaitanyaprem deleted the fix/peer-mgmt-races branch August 16, 2023 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richard-ramos richard-ramos richard-ramos approved these changes

Assignees
No one assigned
Labels
None yet
Projects
Waku
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@chaitanyaprem @status-im-auto @richard-ramos