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

[BUG] vcsim: OptionsManager is racy #2906

Closed
brakthehack opened this issue Jul 19, 2022 · 2 comments · May be fixed by #2907
Closed

[BUG] vcsim: OptionsManager is racy #2906

brakthehack opened this issue Jul 19, 2022 · 2 comments · May be fixed by #2907
Labels
lifecycle/stale

Comments

@brakthehack
Copy link
Contributor

Describe the bug
OptionsManager holds several data structures that can be updated by different threads. Since vcsim uses a single OptionsManager per host, it can race when two threads are updating it at the same time.

To Reproduce

  1. Instantiate vcsim
  2. Have two threads race to update OptionsManager Settings on the same host.
  3. Eventually, observe the race using your favorite tooling.

Expected behavior
No races to update the same data structures in OptionsManager.

Affected version
0.27.4

Screenshots/Debug Output

WARNING: DATA RACE
Read at 0x00c00059c280 by goroutine 72:
  github.com/vmware/govmomi/simulator.(*OptionManager).QueryOptions()
      /home/worker/workspace/pkg/mod/github.com/vmware/[email protected]/simulator/option_manager.go:58 +0x450
  runtime.call16()
      /home/worker/workspace/linux64/src/runtime/asm_amd64.s:625 +0x48
  reflect.Value.Call()
      /home/worker/workspace/linux64/src/reflect/value.go:339 +0xd7
  github.com/vmware/govmomi/simulator.(*Service).call.func1()
      /home/worker/workspace/pkg/mod/github.com/vmware/[email protected]/simulator/simulator.go:218 +0x94
  github.com/vmware/govmomi/simulator.(*Registry).WithLock()
      /home/worker/workspace/pkg/mod/github.com/vmware/[email protected]/simulator/registry.go:625 +0x5e
  github.com/vmware/govmomi/simulator.(*Service).call()
      /home/worker/workspace/pkg/mod/github.com/vmware/[email protected]/simulator/simulator.go:217 +0x1acd
  github.com/vmware/govmomi/simulator.(*Service).ServeSDK()
      /home/worker/workspace/pkg/mod/github.com/vmware/[email protected]/simulator/simulator.go:492 +0xd0d
  github.com/vmware/govmomi/simulator.(*Service).ServeSDK-fm()
      /home/worker/workspace/pkg/mod/github.com/vmware/[email protected]/simulator/simulator.go:446 +0x57
  net/http.HandlerFunc.ServeHTTP()
      /home/worker/workspace/linux64/src/net/http/server.go:2047 +0x4d
  net/http.(*ServeMux).ServeHTTP()
      /home/worker/workspace/linux64/src/net/http/server.go:2425 +0xc5
  net/http.serverHandler.ServeHTTP()
      /home/worker/workspace/linux64/src/net/http/server.go:2879 +0x89a
  net/http.(*conn).serve()
      /home/worker/workspace/linux64/src/net/http/server.go:1930 +0x12e4
  net/http.(*Server).Serve·dwrap·87()
      /home/worker/workspace/linux64/src/net/http/server.go:3034 +0x58

Previous write at 0x00c00059c280 by goroutine 74:
  github.com/vmware/govmomi/simulator.(*OptionManager).UpdateOptions()
      /home/worker/workspace/pkg/mod/github.com/vmware/[email protected]/simulator/option_manager.go:105 +0x2b7
  runtime.call16()
      /home/worker/workspace/linux64/src/runtime/asm_amd64.s:625 +0x48
  reflect.Value.Call()
      /home/worker/workspace/linux64/src/reflect/value.go:339 +0xd7
  github.com/vmware/govmomi/simulator.(*Service).call.func1()
      /home/worker/workspace/pkg/mod/github.com/vmware/[email protected]/simulator/simulator.go:218 +0x94
  github.com/vmware/govmomi/simulator.(*Registry).WithLock()
      /home/worker/workspace/pkg/mod/github.com/vmware/[email protected]/simulator/registry.go:625 +0x5e
  github.com/vmware/govmomi/simulator.(*Service).call()
      /home/worker/workspace/pkg/mod/github.com/vmware/[email protected]/simulator/simulator.go:217 +0x1acd
  github.com/vmware/govmomi/simulator.(*Service).ServeSDK()
      /home/worker/workspace/pkg/mod/github.com/vmware/[email protected]/simulator/simulator.go:492 +0xd0d
  github.com/vmware/govmomi/simulator.(*Service).ServeSDK-fm()
      /home/worker/workspace/pkg/mod/github.com/vmware/[email protected]/simulator/simulator.go:446 +0x57
  net/http.HandlerFunc.ServeHTTP()
      /home/worker/workspace/linux64/src/net/http/server.go:2047 +0x4d
  net/http.(*ServeMux).ServeHTTP()
      /home/worker/workspace/linux64/src/net/http/server.go:2425 +0xc5
  net/http.serverHandler.ServeHTTP()
      /home/worker/workspace/linux64/src/net/http/server.go:2879 +0x89a
  net/http.(*conn).serve()
      /home/worker/workspace/linux64/src/net/http/server.go:1930 +0x12e4
  net/http.(*Server).Serve·dwrap·87()
      /home/worker/workspace/linux64/src/net/http/server.go:3034 +0x58
@github-actions
Copy link
Contributor

Howdy 🖐   brakthehack ! Thank you for your interest in this project. We value your feedback and will respond soon.

If you want to contribute to this project, please make yourself familiar with the CONTRIBUTION guidelines.

brakthehack added a commit to brakthehack/govmomi that referenced this issue Jul 19, 2022
@brakthehack
vcsim: Synchronize calls to OptionsManager internal data structures
14d3cfe 
As described in vmware#2906, `OptionsManager` is racy as calls to its internal
data structures are unsynchronized. When multiple threads are making
calls to `OptionsManager`, they can race against each other.

This change protects these data structures with a mutex, to avoid the
race conditions.

Closes: vmware#2906
@brakthehack brakthehack mentioned this issue Jul 19, 2022
Open
11 tasks
@github-actions
Copy link
Contributor

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Mark as fresh by adding the comment /remove-lifecycle stale.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
lifecycle/stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

vcsim: Synchronize calls to OptionsManager internal data structures brakthehack/govmomi
1 participant
@brakthehack