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] Race in simulator vm.Relocate #3565

Closed
prziborowski opened this issue Sep 27, 2024 · 1 comment · Fixed by #3567
Closed

[BUG] Race in simulator vm.Relocate #3565

prziborowski opened this issue Sep 27, 2024 · 1 comment · Fixed by #3567

Comments

@prziborowski
Copy link
Contributor

Describe the bug
I have noticed a data race when my tests are using the vm.Relocate method for a folder change and for generating the datacenter event argument.

To Reproduce
Steps to reproduce the behavior:

I've tried to make a test case in virtual_machine_test.go, although I'm not sure if it is valid or not (I hit a couple kinds of races when I run):

func TestVmRelocate(t *testing.T) {
   ctx := context.Background()

   m := VPX()
   defer m.Remove()
   err := m.Create()
   if err != nil {
      t.Fatal(err)
   }

   s := m.Service.NewServer()
   defer s.Close()

   c, err := govmomi.NewClient(ctx, s.URL, true)
   if err != nil {
      t.Fatal(err)
   }

   vm := object.NewVirtualMachine(c.Client, Map.Any("VirtualMachine").Reference())

   finder := find.NewFinder(c.Client, false)

   dc, err := finder.DefaultDatacenter(ctx)
   if err != nil {
      t.Fatal(err)
   }

   finder.SetDatacenter(dc)
   folders, err := dc.Folders(ctx)
   if err != nil {
      t.Fatal(err)
   }

   spec := types.VirtualMachineRelocateSpec{}
   vmFolder := folders.VmFolder
   for i := 0; i < 10; i++ {
      vmFolderRef := vmFolder.Reference()
      spec.Folder = &vmFolderRef
      go func() {
         task, err := vm.Relocate(ctx, spec, types.VirtualMachineMovePriorityDefaultPriority)
         if err != nil {
            t.Fatal("Failed to relocate", err)
         }
         _ = task.Wait(ctx)
      }()
      vmFolder, err = vmFolder.CreateFolder(ctx, "child")
      if err != nil {
         t.Fatal("Failed to create folder", err)
      }
   }
}

When I run go test -race -count=10 -run TestVmRelocate ./...
That tends to fail with either:

==================
WARNING: DATA RACE
Read at 0x00c000102468 by goroutine 597:
  github.com/vmware/govmomi/simulator.(*Registry).getEntityParent()
      /Users/nathanp/govmomi/simulator/registry.go:340 +0x87
  github.com/vmware/govmomi/simulator.(*Registry).getEntityDatacenter()
      /Users/nathanp/govmomi/simulator/registry.go:354 +0x72
  github.com/vmware/govmomi/simulator.datacenterEventArgument()
      /Users/nathanp/govmomi/simulator/datacenter.go:142 +0x45
  github.com/vmware/govmomi/simulator.(*VirtualMachine).RelocateVMTask.func1()
      /Users/nathanp/govmomi/simulator/virtual_machine.go:2504 +0x128d
  github.com/vmware/govmomi/simulator.(*Task).Run.func1()
      /Users/nathanp/govmomi/simulator/task.go:130 +0x13b

Previous write at 0x00c000102468 by goroutine 599:
  github.com/vmware/govmomi/simulator.(*Registry).PutEntity()
      /Users/nathanp/govmomi/simulator/registry.go:183 +0x78
  github.com/vmware/govmomi/simulator.folderPutChild.func1()
      /Users/nathanp/govmomi/simulator/folder.go:94 +0x292
  github.com/vmware/govmomi/simulator.(*Registry).WithLock()
      /Users/nathanp/govmomi/simulator/registry.go:658 +0x58
  github.com/vmware/govmomi/simulator.(*Context).WithLock()
      /Users/nathanp/govmomi/simulator/session_manager.go:415 +0xd8
  github.com/vmware/govmomi/simulator.folderPutChild()
      /Users/nathanp/govmomi/simulator/folder.go:91 +0x30
  github.com/vmware/govmomi/simulator.(*Folder).MoveIntoFolderTask.func1()
      /Users/nathanp/govmomi/simulator/folder.go:574 +0xcd
  github.com/vmware/govmomi/simulator.(*Task).Run.func1()
      /Users/nathanp/govmomi/simulator/task.go:130 +0x13b

Or

==================
WARNING: DATA RACE
Write at 0x000011f91d00 by goroutine 681:
  github.com/vmware/govmomi/simulator.NewServiceInstance()
      /Users/nathanp/govmomi/simulator/service_instance.go:39 +0x224
  github.com/vmware/govmomi/simulator.(*Model).Create()
      /Users/nathanp/govmomi/simulator/model.go:483 +0x144
  github.com/vmware/govmomi/simulator.TestVmRelocate()
      /Users/nathanp/govmomi/simulator/virtual_machine_test.go:1837 +0x3a6
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1689 +0x21e
  testing.(*T).Run.gowrap1()
      /usr/local/go/src/testing/testing.go:1742 +0x44

Previous read at 0x000011f91d00 by goroutine 653:
  github.com/vmware/govmomi/simulator.datacenterEventArgument()
      /Users/nathanp/govmomi/simulator/datacenter.go:142 +0x50
  github.com/vmware/govmomi/simulator.(*VirtualMachine).RelocateVMTask.func1()
      /Users/nathanp/govmomi/simulator/virtual_machine.go:2504 +0x128d
  github.com/vmware/govmomi/simulator.(*Task).Run.func1()
      /Users/nathanp/govmomi/simulator/task.go:130 +0x13b

Expected behavior
I don't expect a data race of the original (or first test reproduction). The latter one is maybe because each model is clobbering the other one when run concurrently? (just guessing)

Affected version
Please provide details on the version used, e.g. release tag, commit, module version, etc.
govmomi@v0.43.0

Screenshots/Debug Output
If applicable, add screenshots or debug output to help explain your problem.

This was the frames for the data race of the original reproduction

==================
WARNING: DATA RACE
Write at 0x00c000bc9268 by goroutine 898:
  github.com/vmware/govmomi/simulator.(*Registry).PutEntity()
      /go/pkg/mod/github.com/vmware/govmomi@v0.43.0/simulator/registry.go:183 +0x78
  github.com/vmware/govmomi/simulator.folderPutChild.func1()
      /go/pkg/mod/github.com/vmware/govmomi@v0.43.0/simulator/folder.go:94 +0x292
  github.com/vmware/govmomi/simulator.(*Registry).WithLock()
      /go/pkg/mod/github.com/vmware/govmomi@v0.43.0/simulator/registry.go:658 +0x58
  github.com/vmware/govmomi/simulator.(*Context).WithLock()
      /go/pkg/mod/github.com/vmware/govmomi@v0.43.0/simulator/session_manager.go:415 +0xd8
  github.com/vmware/govmomi/simulator.folderPutChild()
      /go/pkg/mod/github.com/vmware/govmomi@v0.43.0/simulator/folder.go:91 +0x30
  github.com/vmware/govmomi/simulator.(*Folder).MoveIntoFolderTask.func1()
      /go/pkg/mod/github.com/vmware/govmomi@v0.43.0/simulator/folder.go:574 +0xcd
  github.com/vmware/govmomi/simulator.(*Task).Run.func1()
      /go/pkg/mod/github.com/vmware/govmomi@v0.43.0/simulator/task.go:130 +0x13b

Previous read at 0x00c000bc9268 by goroutine 897:
  github.com/vmware/govmomi/simulator.(*Registry).getEntityParent()
      /go/pkg/mod/github.com/vmware/govmomi@v0.43.0/simulator/registry.go:340 +0x87
  github.com/vmware/govmomi/simulator.(*Registry).getEntityDatacenter()
      /go/pkg/mod/github.com/vmware/govmomi@v0.43.0/simulator/registry.go:354 +0x72
  github.com/vmware/govmomi/simulator.datacenterEventArgument()
      /go/pkg/mod/github.com/vmware/govmomi@v0.43.0/simulator/datacenter.go:142 +0x45
  github.com/vmware/govmomi/simulator.(*VirtualMachine).RelocateVMTask.func1()
      /go/pkg/mod/github.com/vmware/govmomi@v0.43.0/simulator/virtual_machine.go:2504 +0x128d
  github.com/vmware/govmomi/simulator.(*Task).Run.func1()
      /go/pkg/mod/github.com/vmware/govmomi@v0.43.0/simulator/task.go:130 +0x13b

Goroutine 898 (running) created at:
  github.com/vmware/govmomi/simulator.(*Task).Run()
      /go/pkg/mod/github.com/vmware/govmomi@v0.43.0/simulator/task.go:125 +0x724
  github.com/vmware/govmomi/simulator.(*Folder).MoveIntoFolderTask()
      /go/pkg/mod/github.com/vmware/govmomi@v0.43.0/simulator/folder.go:582 +0x157
  github.com/vmware/govmomi/simulator.(*VirtualMachine).RelocateVMTask.func1()
      /go/pkg/mod/github.com/vmware/govmomi@v0.43.0/simulator/virtual_machine.go:2491 +0xf44
  github.com/vmware/govmomi/simulator.(*Task).Run.func1()
      /go/pkg/mod/github.com/vmware/govmomi@v0.43.0/simulator/task.go:130 +0x13b

Goroutine 897 (running) created at:
  github.com/vmware/govmomi/simulator.(*Task).Run()
      /go/pkg/mod/github.com/vmware/govmomi@v0.43.0/simulator/task.go:125 +0x724
  github.com/vmware/govmomi/simulator.(*VirtualMachine).RelocateVMTask()
      /go/pkg/mod/github.com/vmware/govmomi@v0.43.0/simulator/virtual_machine.go:2515 +0x157
  runtime.call32()
      /usr/local/go/src/runtime/asm_amd64.s:771 +0x42
  reflect.Value.Call()
      /usr/local/go/src/reflect/value.go:380 +0xb5
  github.com/vmware/govmomi/simulator.(*Service).call.func1()
      /go/pkg/mod/github.com/vmware/govmomi@v0.43.0/simulator/simulator.go:217 +0x87
  github.com/vmware/govmomi/simulator.(*Registry).WithLock()
      /go/pkg/mod/github.com/vmware/govmomi@v0.43.0/simulator/registry.go:658 +0x58
  github.com/vmware/govmomi/simulator.(*Service).call()
      /go/pkg/mod/github.com/vmware/govmomi@v0.43.0/simulator/simulator.go:216 +0x1bf2
  github.com/vmware/govmomi/simulator.(*Service).ServeSDK()
      /go/pkg/mod/github.com/vmware/govmomi@v0.43.0/simulator/simulator.go:501 +0xe4a
  github.com/vmware/govmomi/simulator.(*Service).ServeSDK-fm()
      <autogenerated>:1 +0x51
  net/http.HandlerFunc.ServeHTTP()
      /usr/local/go/src/net/http/server.go:2171 +0x47
  net/http.(*ServeMux).ServeHTTP()
      /usr/local/go/src/net/http/server.go:2688 +0x1ef
  net/http.serverHandler.ServeHTTP()
      /usr/local/go/src/net/http/server.go:3142 +0x2a1
  net/http.(*conn).serve()
      /usr/local/go/src/net/http/server.go:2044 +0x13c4
  net/http.(*Server).Serve.gowrap3()
      /usr/local/go/src/net/http/server.go:3290 +0x4f
==================

Additional context
Add any other context about the problem here.
@github-actions GitHub Actions
Copy link
Contributor

Howdy 🖐   prziborowski ! 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.

@dougm dougm mentioned this issue Sep 30, 2024
Merged
@dougm dougm closed this as completed in 6cea86e Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

vcsim: fix RelocateVM_Task related races dougm/govmomi
1 participant
@prziborowski