Add mutex and test for safe concurrent VBoxManage access

[nathanleclaire]

Closes #2154

Signed-off-by: Nathan LeClaire nathan.leclaire@gmail.com

[nathanleclaire]

Like I said I think it may allow us to also remove the serial bit in runActionForeachMachine as well. We'll see.

[nathanleclaire]

@docker/machine-maintainers PTAL

[dmp42]

This is tripping a race condition apparently.

[nathanleclaire]

Yeah, I noticed that. I'm actually not sure how much good the test is doing in this case (since essentially it's elaborate test of mutex itself), so I might just take it out, even though I initially was really adamant about including one.

[nathanleclaire]

Alright, how about this: I've turned off running this test on go test -race using build tag exclusion. I want to still have the test run in CI, however, so I also added a line to test-long target in Makefile to also run go test without -race. The overhead is not really substantial as far as I can tell (a few extra seconds). This seems to be a reasonable compromise to me.

[nathanleclaire]

To be clear, it's intentional that the test races.

[dmp42]

however, so I also added a line to test-long target in Makefile to also run go test without -race

Ah - I don't think you need this actually - test-short will already run your test (unless your test is specifically meant to run only on long?).

@docker/machine-maintainers PTAL

[nathanleclaire]

Right, I see. I'll take the Makefile change out then. Should be good to go since you have to intentionally mark tests which will get skipped with short, which I haven't done here.

[dgageot]

@nathanleclaire I've tested the code with #2154 and it does not solve the issue.

Reopening #2172 which fixes the #2154 issue.

[dgageot]

@nathanleclaire shall we close this PR? Clearly, the mutex has no effect since each virtualbox driver is running in its own process.

[nathanleclaire]

@dgageot Yep