[CURATOR-535] TestServer random port selection has a race condition

[paul8263]

This PR is to mitigate the race condition issue. The random available port is now assigned right before starting Zookeeper testing server.

[tisonkun]

It seems that with this patch it's still possible that a resolved port be in used by other process before it's used by the caller.

[paul8263]

It seems that with this patch it's still possible that a resolved port be in used by other process before it's used by the caller.

Yes. currently this PR updates InstanceSpec so that the random port will only be allocated when it is actually used.
But There are still some more steps before the port is actually used: wrap the configuration into QuorumPeerConfig and start the server. There could be a race condition in these steps.

QuorumPeerConfig config = configBuilder.buildConfig(thisInstanceIndex);
main.runFromConfig(config);

I suggest we could introduce a file lock in order to alleviate the race condition among processes. But the implementation might be cumbersome.

[tisonkun]

Since this patch doesn't resolve the problem, I tend not to merge such change.

Perhaps @eolivelli @Randgalt have different ideas.

[madrob]

apache/zookeeper#1868 May be relevant

[paul8263]

Hi @tisonkun @eolivelli and @Randgalt ,
Thank you for your reply. This problem is unusual. I got this problem when running unit tests in other project which relies on Zookeeper. The unit tests are running parallelly so that TestServer creating process might get a race condition when allocating unused ports. Currently my solution is the steps below:

1. Get a random unused port.
2. Implementing a file lock.
3. Allocating the port to TestServer.
4. Check if TestServer starts properly. If it starts successfully, release the file lock.
   I would like to move those steps inside TestServer creation and start process. However I worry that introducing a file lock might not be the best solution as it only solves an unusual problem at the cost of performance degradation.
   Have you got any better ideas? I think using the file lock should be considered as the last resort. Correct me if I am wrong.

[tisonkun]

Should be fixed by 7e7c207 already.