Skip to content

Fix memory leak in the p2p channel - #4801

Merged
soffokl merged 5 commits into
masterfrom
fix-p2p-memory-leak
Feb 22, 2022
Merged

Fix memory leak in the p2p channel#4801
soffokl merged 5 commits into
masterfrom
fix-p2p-memory-leak

Conversation

@soffokl

@soffokl soffokl commented Feb 21, 2022

Copy link
Copy Markdown
Member

No description provided.

@Snawoot

Snawoot commented Feb 21, 2022

Copy link
Copy Markdown
Contributor

note data race and panic:

==================
WARNING: DATA RACE
Write at 0x00c000408028 by goroutine 30:
  github.com/mysteriumnetwork/node/p2p.(*channel).Close.func1()
      /home/gitlab-runner/go/src/github.com/mysteriumnetwork/node/p2p/channel.go:493 +0x829
  sync.(*Once).doSlow()
      /usr/local/go/src/sync/once.go:68 +0x127
  sync.(*Once).Do()
      /usr/local/go/src/sync/once.go:59 +0x46
  github.com/mysteriumnetwork/node/p2p.(*channel).Close()
      /home/gitlab-runner/go/src/github.com/mysteriumnetwork/node/p2p/channel.go:462 +0x111
  github.com/mysteriumnetwork/node/p2p.TestChannel_Send_To_When_Peer_Starts_Later()
      /home/gitlab-runner/go/src/github.com/mysteriumnetwork/node/p2p/channel_test.go:224 +0x188
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1259 +0x22f
  testing.(*T).Run·dwrap·21()
      /usr/local/go/src/testing/testing.go:1306 +0x47
Previous read at 0x00c000408028 by goroutine 35:
  github.com/mysteriumnetwork/node/p2p.(*channel).localSendLoop()
      /home/gitlab-runner/go/src/github.com/mysteriumnetwork/node/p2p/channel.go:368 +0x5e
  github.com/mysteriumnetwork/node/p2p.(*channel).launchReadSendLoops·dwrap·6()
      /home/gitlab-runner/go/src/github.com/mysteriumnetwork/node/p2p/channel.go:251 +0x39
Goroutine 30 (running) created at:
  testing.(*T).Run()
      /usr/local/go/src/testing/testing.go:1306 +0x726
  testing.runTests.func1()
      /usr/local/go/src/testing/testing.go:1598 +0x99
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1259 +0x22f
  testing.runTests()
      /usr/local/go/src/testing/testing.go:1596 +0x7ca
  testing.(*M).Run()
      /usr/local/go/src/testing/testing.go:1504 +0x9d1
  main.main()
      _testmain.go:129 +0x324
Goroutine 35 (finished) created at:
  github.com/mysteriumnetwork/node/p2p.(*channel).launchReadSendLoops()
      /home/gitlab-runner/go/src/github.com/mysteriumnetwork/node/p2p/channel.go:251 +0x1e4
  github.com/mysteriumnetwork/node/p2p.createTestChannels()
      /home/gitlab-runner/go/src/github.com/mysteriumnetwork/node/p2p/channel_test.go:338 +0x4a4
  github.com/mysteriumnetwork/node/p2p.TestChannel_Send_To_When_Peer_Starts_Later()
      /home/gitlab-runner/go/src/github.com/mysteriumnetwork/node/p2p/channel_test.go:214 +0x44
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1259 +0x22f
  testing.(*T).Run·dwrap·21()
      /usr/local/go/src/testing/testing.go:1306 +0x47
==================
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xc56482]
goroutine 86 [running]:
github.com/mysteriumnetwork/node/p2p.reopenChannel(0xc000408000)
	/home/gitlab-runner/go/src/github.com/mysteriumnetwork/node/p2p/channel_test.go:282 +0x62
github.com/mysteriumnetwork/node/p2p.TestChannel_Send_To_When_Peer_Starts_Later.func1()
	/home/gitlab-runner/go/src/github.com/mysteriumnetwork/node/p2p/channel_test.go:228 +0x65
created by github.com/mysteriumnetwork/node/p2p.TestChannel_Send_To_When_Peer_Starts_Later
	/home/gitlab-runner/go/src/github.com/mysteriumnetwork/node/p2p/channel_test.go:226 +0x279```

@soffokl
soffokl force-pushed the fix-p2p-memory-leak branch from f1e7015 to eefc69c Compare February 22, 2022 03:03
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

Merging #4801 (f1e7015) into master (8bfa049) will decrease coverage by 0.02%.
The diff coverage is 80.76%.

❗ Current head f1e7015 differs from pull request most recent head eefc69c. Consider uploading reports for the commit eefc69c to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4801      +/-   ##
==========================================
- Coverage   39.26%   39.24%   -0.03%     
==========================================
  Files         356      356              
  Lines       19136    19138       +2     
==========================================
- Hits         7514     7510       -4     
- Misses      10899    10903       +4     
- Partials      723      725       +2     
Impacted Files Coverage Δ
nat/traversal/pinger.go 77.51% <ø> (-0.39%) ⬇️
p2p/channel.go 66.06% <80.76%> (-2.32%) ⬇️
session/pingpong/hermes_promise_handler.go 59.29% <0.00%> (+0.77%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8bfa049...eefc69c. Read the comment docs.

@soffokl
soffokl merged commit 1a14ae5 into master Feb 22, 2022
@soffokl
soffokl deleted the fix-p2p-memory-leak branch February 22, 2022 03:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants