vcsim: Synchronize calls to OptionsManager internal data structures

[brakthehack]

Description

As described in #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: #2906

Type of change

Please mark options that are relevant:

• [x] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
• [ ] This change requires a documentation update
• [ ] Build related change

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. If applicable, please also list any relevant details for your test configuration.

Wrote a simple test and ran it with the -race flag. I consistently hit race conditions without my change and none with my change:

Snip:

	vcsim := vcsimulator.NewSimulator()
	defer vcsim.Stop()

	vcsim2 := vcsimulator.NewSimulator()
	defer vcsim2.Stop()

	t.Run("test", func(t *testing.T) {
		client := vcsim.Client.Client
		client2 := vcsim2.Client.Client
		m := object.NewOptionManager(client, client.ServiceContent.Setting.Reference())
		m2 := object.NewOptionManager(client2, client2.ServiceContent.Setting.Reference())
		ctx, cancel := context.WithTimeout(context.Background(), time.Second*2)
		defer cancel()
		for {
			select {
			case <-ctx.Done():
				return
			default:
			}
			// adding the new options to the simulated VC
			opt := fmt.Sprintf("config.%d", rand.Int())
			go m.Update(ctx, []vimtypes.BaseOptionValue{
				&vimtypes.OptionValue{Key: opt, Value: "bar"},
			})

			// adding the new options to the simulated VC
			opt2 := fmt.Sprintf("config.%d", rand.Int())
			go m2.Update(ctx, []vimtypes.BaseOptionValue{
				&vimtypes.OptionValue{Key: opt2, Value: "bar"},
			})

			m.Query(ctx, opt)
		}

Checklist:

• [x] My code follows the CONTRIBUTION guidelines of this project
• [ ] I have commented my code, particularly in hard-to-understand areas
• [ ] I have made corresponding changes to the documentation
• [ ] I have added tests that prove my fix is effective or that my feature works
• [ ] New and existing unit tests pass locally with my changes
• [ ] Any dependent changes have been merged

[github-actions[bot]]

This Pull Request 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.

[akutz]

For some reason @dougm 's comments didn't show up when I read this PR. GH must have been brain-farting, because I swore this PR showed up as "new."

Anyway, Doug is correct in that each MO has its own lock domain, so my suggestions should not actually be necessary. Still, they're worth considering as general guidelines for handling concurrency.