Understanding CompletableFuture simple implementation

Viewed 162

Consider the following code taken from this article. It implements something similar to a CompletableFuture for learning purposes.

Here's the get() function of WaitingFuture:

@Override
public V get() throws ExecutionException {
    this.thread = Thread.currentThread();
    LockSupport.park(this);

    if (throwable != null) {
        throw new ExecutionException(throwable);
    }

    return result;
}

And here's the run() function of RunnableWaitingFuture:

@Override
    public void run() {
            try {
                waitingFuture.result = userFunction.get();
            } catch (Throwable throwable) {
                waitingFuture.throwable = throwable;
            } finally {
                waitingFuture.finished = true;
                LockSupport.unpark(waitingFuture.thread);
            }
        }
    }

The Question:
It seems to me that if run() will finish before get() is even called then LockSupport.park(this); will be called after LockSupport.unpark(waitingFuture.thread), leaving the thread parking forever.

Is that true?

2 Answers

park()/unpark() is different to wait/notify, as the signal won’t be lost if unpark has been called before park().

However, it’s still only a single bit that doesn’t count how often unpark has been called, so it’s still wrong to assume that all calls will be perfectly paired.

Further, park will silently return on interrupts and is even allowed to return spuriously, which means for no reason.

In other words, even returning from park() doesn’t guaranty that the condition has been fulfilled. Just like with the other notification mechanisms, there is no way around using it in a loop.

The cited code is even worse, as it has another race condition regarding the thread variable. There is no guaranty that it has been written at the point where the other thread reads it for notifying it.

yes.

LockSupport.park(this);

should be replaced with something like

while (!waitingFuture.finished) {
    LockSupport.park(this);
}

Generally, LockSupport.park is too low a feature, and Object::wait or Condition::await should be used instead for reliability.

Related