Is this potentially racy? We're testing that the lock acquire works but theoretically that might not happen if the first coderd grabs the job, completes it, and then the second one does.

I doubt it matters as I suppose we're happy even if the try acquire is hit only a faction of the time, but thought I'd flag it anyway.

Looking again, you're probably right. I ran the test with verbose logging and it looks like this all occurs within0.05s.

If the testdoesn't hit the lock, then we are likely to hit a flake. I'll have a go at increasing this time buffer.

I think you might be able to reduce (but not eliminate) racyness by having a secondchan struct{} that you then close after starting both goroutines, making them both wait until it's closed to start.

e.g.

    startCh := make(chan struct{})go func() {        <-startChtickChA <- nextclose(tickChA)}()go func() {        <-startChtickChB <- nextclose(tickChB)}()    close(startCh)

You might also be able to get both of them to tick very closely in time by sharing the same tick channel, and making it buffered with size 2. (Of course then you'd want to avoid closing the channel twice to avoid a panic)

I've gone with your proposal@johnstcn.

It looks like for testing we just use an echo provisioner job, so getting that to take artificially longer for this specific test may not be a trivial task.

I think you might be able to reduce (but not eliminate) racyness by having a secondchan struct{} that you then close after starting both goroutines, making them both wait until it's closed to start.

e.g.

    startCh := make(chan struct{})go func() {        <-startChtickChA <- nextclose(tickChA)}()go func() {        <-startChtickChB <- nextclose(tickChB)}()    close(startCh)

You might also be able to get both of them to tick very closely in time by sharing the same tick channel, and making it buffered with size 2. (Of course then you'd want to avoid closing the channel twice to avoid a panic)