CompletionListenerFuture wakeup for multiple threads, protect against spurious wakeup

[cruftex]

From CompletionListenerFuture:

public void onCompletion() throws IllegalStateException {
  synchronized (this) {
    if (isCompleted) {
      throw new IllegalStateException("Attempted to use a CompletionListenerFuture instance more than once");
    } else {
      isCompleted = true;
      notify();
    }
  }
}

This is notifying only one thread upon completion, all others block indefinitely.

Solution: notify() -> notifyAll()

From: #348

The following code fails to deal with spurious wakeup. It is allowed that the thread is notified without a reason. To deal with this situation, the if(!completed) should be while(!completed). Of course the remaining timeout needs to be corrected.

 @Override
  public Void get(long timeout, TimeUnit unit) throws InterruptedException, ExecutionException, TimeoutException {
    synchronized (this) {
      if (!isCompleted) {
        unit.timedWait(this, timeout);
      }

      if (isCompleted) {
        if (exception == null) {
          return null;
        } else {
          throw new ExecutionException(exception);
        }
      } else {
        throw new TimeoutException();
      }
    }
  }

[pveentjer]

👍 it should indeed be notifyAll.

[cruftex]

There are three findings in the CompletionListenerFuture at the moment. Additional ones:

#348
#346

Will probably bundle this.

[cruftex]

Pull request is ready.

Another Pull Request against the TCK is extending the tests to better check the current behavior so we don't introduce an regression. A test for the bug is also included but optional. See PR comment.

[cruftex]

The PR on the CompletionListenerFuture also does minor corrections in the JavaDoc, since the existing one was stating that a value is returned.

[gregrluck]

Closed with PR 371