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

[Race Detector] Race in proxylib package between (*AccessLogServer).Close() and StartAccessLogServer.func1() #16371

Closed
jibi opened this issue May 31, 2021 · 1 comment · Fixed by #17141
Closed

[Race Detector] Race in proxylib package between (*AccessLogServer).Close() and StartAccessLogServer.func1() #16371

jibi opened this issue May 31, 2021 · 1 comment · Fixed by #17141
Labels
area/CI Continuous Integration testing issue or flake area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. ci/flake This is a known failure that occurs in the tree. Please investigate me! kind/bug/race-detector stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale.

Comments

@jibi
Copy link
Member

jibi commented May 31, 2021

Found in Travis: https://travis-ci.com/github/cilium/cilium/jobs/509157458

Read at 0x00c000502128 by goroutine 99:
  github.com/cilium/cilium/proxylib/test.(*AccessLogServer).Close()
      /home/travis/gopath/src/github.com/cilium/cilium/proxylib/test/accesslog_server.go:48 +0xbc
  github.com/cilium/cilium/proxylib.TestTwoRulesOnSamePortFirstNoL7()
      /home/travis/gopath/src/github.com/cilium/cilium/proxylib/proxylib_test.go:446 +0x265
  testing.tRunner()
      /home/travis/.gimme/versions/go1.16.4.linux.amd64/src/testing/testing.go:1193 +0x202

Previous write at 0x00c000502128 by goroutine 100:
  github.com/cilium/cilium/proxylib/test.StartAccessLogServer.func1()
      /home/travis/gopath/src/github.com/cilium/cilium/proxylib/test/accesslog_server.go:116 +0x1ed

Goroutine 99 (running) created at:
  testing.(*T).Run()
      /home/travis/.gimme/versions/go1.16.4.linux.amd64/src/testing/testing.go:1238 +0x5d7
  testing.runTests.func1()
      /home/travis/.gimme/versions/go1.16.4.linux.amd64/src/testing/testing.go:1511 +0xa6
  testing.tRunner()
      /home/travis/.gimme/versions/go1.16.4.linux.amd64/src/testing/testing.go:1193 +0x202
  testing.runTests()
      /home/travis/.gimme/versions/go1.16.4.linux.amd64/src/testing/testing.go:1509 +0x612
  testing.(*M).Run()
      /home/travis/.gimme/versions/go1.16.4.linux.amd64/src/testing/testing.go:1417 +0x3b3
  main.main()
      _testmain.go:291 +0x356

Goroutine 100 (finished) created at:
  github.com/cilium/cilium/proxylib/test.StartAccessLogServer()
      /home/travis/gopath/src/github.com/cilium/cilium/proxylib/test/accesslog_server.go:99 +0x42f
  github.com/cilium/cilium/proxylib.TestTwoRulesOnSamePortFirstNoL7()
      /home/travis/gopath/src/github.com/cilium/cilium/proxylib/proxylib_test.go:415 +0x66
  testing.tRunner()
      /home/travis/.gimme/versions/go1.16.4.linux.amd64/src/testing/testing.go:1193 +0x202
@jibi jibi added area/CI Continuous Integration testing issue or flake area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. ci/flake This is a known failure that occurs in the tree. Please investigate me! kind/bug/race-detector labels May 31, 2021
@stale
Copy link

stale bot commented Jul 30, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Jul 30, 2021
@gandro gandro mentioned this issue Aug 11, 2021
Merged
gandro added a commit to gandro/cilium that referenced this issue Aug 12, 2021
@gandro
proxylib: Fix race in accesslog_server
cd4a4c2 
This introduces a mutex for the connections list, as there is a race
where `Close()` starts iterating over the connections while the accept
loop is trying to append a new connection to it.

Fixes: cilium#16371

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
tklauser pushed a commit that referenced this issue Aug 19, 2021
@gandro @tklauser
proxylib: Fix race in accesslog_server
82ec9f0 
This introduces a mutex for the connections list, as there is a race
where `Close()` starts iterating over the connections while the accept
loop is trying to append a new connection to it.

Fixes: #16371

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
tklauser pushed a commit to tklauser/cilium that referenced this issue Aug 23, 2021
@gandro @tklauser
proxylib: Fix race in accesslog_server
8d51636 
[ upstream commit 82ec9f0 ]

This introduces a mutex for the connections list, as there is a race
where `Close()` starts iterating over the connections while the accept
loop is trying to append a new connection to it.

Fixes: cilium#16371

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Signed-off-by: Tobias Klauser <tobias@cilium.io>
tklauser pushed a commit that referenced this issue Aug 24, 2021
@gandro @tklauser
proxylib: Fix race in accesslog_server
9104a49 
[ upstream commit 82ec9f0 ]

This introduces a mutex for the connections list, as there is a race
where `Close()` starts iterating over the connections while the accept
loop is trying to append a new connection to it.

Fixes: #16371

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Signed-off-by: Tobias Klauser <tobias@cilium.io>
krishgobinath pushed a commit to krishgobinath/cilium that referenced this issue Oct 20, 2021
@gandro
proxylib: Fix race in accesslog_server
5128f1c 
This introduces a mutex for the connections list, as there is a race
where `Close()` starts iterating over the connections while the accept
loop is trying to append a new connection to it.

Fixes: cilium#16371

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/CI Continuous Integration testing issue or flake area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. ci/flake This is a known failure that occurs in the tree. Please investigate me! kind/bug/race-detector stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

proxylib: Fix data races in unit tests gandro/cilium
1 participant
@jibi