3
\$\begingroup\$

Introduction

I've created a typed Promise implementation in Java for Android, borrowing ideas from this article for a JavaScript implementation, by Forbes Lindesay.

To allow for "asynchronicity", my implementation posts callbacks (Promise.Callback) to the message queue of main thread (Looper.getMainLooper()). I don't know the appropriate terminology, but in this case by asynchronicity I mean the following:

new Promise<>(new Promise.Executor<Integer>() {
    @Override
    public void execute(Promise.Action<Integer> action) {
        // just simple synchronous code:
        System.out.println("This is executed first.");
        action.resolve(1);
    }
})
.andThen(new Promise.Callback<Integer, Void>() {
    @Override
    public Promise.Result<Void> execute(Integer value) {
        System.out.println("This is executed third. Got: " + value);
        return null;
    }
});

System.out.println("This is executed second.");

In other words: the executor (constructor argument) is executed first, but the callback passed to the Promise<T>.andThen() method is only executed after the main code block (with System.out.println("This is executed second.");) has finished, even though the code in the executor is synchronous. I believe this to be in line with how JavaScript promises behave.

To provide for this asynchronicity, callbacks are posted on the message queue of main thread (simply with a delay of 0 milliseconds and utilizing Looper.getMainLooper()), hence it currently only runs on Android.

I also thought about posting callbacks to any possibly other currently executing thread, but those are not guaranteed to have a Looper nor guaranteed to still be alive and I didn't want to make it too complicated, nor did I want to have to create Threads myself, since I understand those to be expensive. I wanted to keep it snappy.

Here are the concerns I'd like to see addressed:

  1. I claim my implementation is thread-safe, but I'm not entirely sure whether I should be using the volatile keyword for certain properties. In particular the private Promise.state. Do you see any other thread-safety issues?
  2. Do you see any issues that could arise from posting the callbacks on the main thread? I believe the actual bodies of callbacks posted on another thread will still be executed in that other thread's context, even though I post them to the main thread, but I'm not entirely sure (of possible edge cases).

Code

On Github

package nl.codifier.android.promise;

import android.os.Handler;
import android.os.Looper;

import androidx.annotation.NonNull;

import java.util.ArrayList;
import java.util.List;

/**
 * This is the {@code Promise} class.
 *
 * @param <T> The type with which the {@code Promise} will be fulfilled
 */
@SuppressWarnings({"WeakerAccess", "UnusedReturnValue", "unused"})
public class Promise<T> {

    /**
     * The object that is passed to the {@link Promise.Executor} function. This object provides the
     * methods to resolve its associated {@code Promise} with.
     *
     * <p>Since a {@code Promise} must be allowed to resolve to either a value of type {@code R}
     * or a new {@code Promise} of type {@code R}, this object contains three overloaded
     * implementations of {@code Promise.Action.resolve()}:</p>
     *
     * <ul>
     *     <li>{@link Promise.Action#resolve(Object)}</li>
     *     <li>{@link Promise.Action#resolve(Promise)}</li>
     *     <li>{@link Promise.Action#resolve(Promise.Result)}</li>
     * </ul>
     *
     * <p>The last one is a convenience method to allow for {@link Promise.Result} as well</p>
     *
     * @param <T> The type with which the associated {@code Promise} will be fulfilled
     * @see Promise#Promise(Executor)
     * @see Promise.Result
     */
    public final static class Action<T> {

        private final Promise<T> promise;

        /**
         * Constructs a {@code Promise.Action} with a {@code Promise} of type {@code T}.
         *
         * @param promise the {@code Promise} of type {@code T} that this {@code Promise.Action} is associated with
         */
        private Action(final Promise<T> promise) {
            this.promise = promise;
        }

        /**
         * Resolves the associated {@code Promise} with {@code T value}.
         *
         * @param value a value of type {@code T}
         */
        public void resolve(final T value) {
            promise.resolveInternal(new Result<>(value));
        }

        /**
         * Resolves the associated {@code Promise} with another {@code Promise<T>}.
         *
         * @param value a {@code Promise} of type {@code T}
         */
        @SuppressWarnings("ConstantConditions")
        public void resolve(@NonNull final Promise<T> value) {
            if(value == null) {
                throw new NullPointerException("Argument 'Promise<T> value' must not be null.");
            }

            promise.resolveInternal(new Result<>(value));
        }

        /**
         * Resolves the associated {@code Promise} with the value wrapped inside {@link Promise.Result}
         *
         * @param value a {@code Promise.Result} of type {@code T}
         * @see Promise.Result
         */
        public void resolve(final Result<T> value) {
            promise.resolveInternal(value);
        }

        /**
         * Rejects the associated {@code Promise} with a {@code Throwable}
         *
         * @param value a {@code Throwable}
         */
        public void reject(final Throwable value) {
            promise.rejectInternal(value);
        }
    }

    /**
     * A wrapper object that {@link Promise.Callback} can optionally return to allow returning
     * either a value of type {@code R} or a {@code Promise} of type {@code R}.
     *
     * <p>A {@code Promise.Callback} must be allowed to return either a value of type {@code R}
     * or a new {@code Promise} of type {@code R}. However, since Java does not allow
     * return types to be this ambiguous (without the need for casting), {@code Promise.Callback}
     * must wrap the return value in this {@code Promise.Result} (unless it does not want to return
     * anything, in which case {@code null} must be returned).</p>
     *
     * @param <R> The type with which the earlier returned {@code Promise} will be fulfilled
     * @see Promise.Callback
     */
    public final static class Result<R> {

        /**
         * The possible wrapped result types of {@code Promise.Result}.
         */
        private enum Type {
            VALUE,
            PROMISE
        }

        /**
         * The wrapped result type of this {@code Promise.Result}.
         */
        private final Type type;

        /**
         * The value of type {@code R} with which this {@code Promise.Result} might be constructed.
         */
        private R value;

        /**
         * The {@code Promise} of type {@code R} with which this {@code Promise.Result} might be constructed.
         */
        private Promise<R> promise;

        /**
         * Constructs a {@code Promise.Result} with a value of type {@code R}.
         *
         * @param value the value of type {@code R} this {@code Promise.Result} wraps
         */
        public Result(final R value) {
            this.value = value;
            this.type = Type.VALUE;
        }

        /**
         * Constructs a {@code Promise.Result} with a {@code Promise} of type {@code R}.
         *
         * @param promise the {@code Promise} of type {@code R}  this {@code Promise.Result} wraps
         */
        @SuppressWarnings("ConstantConditions")
        public Result(@NonNull final Promise<R> promise) {
            if(promise == null) {
                throw new NullPointerException("Constructor argument 'Promise<R> promise' must not be null.");
            }

            this.promise = promise;
            this.type = Type.PROMISE;
        }

        /**
         * Getter to determine the type of the wrapped object.
         *
         * @return the {@code Promise.Result.Type}
         */
        private Type getType() {
            return type;
        }

        /**
         * Getter to retrieve the wrapped value.
         *
         * @return the value of type {@code R}
         */
        private R getValue() {
            return value;
        }

        /**
         * Getter to retrieve the wrapped {@code Promise}.
         *
         * @return the {@code Promise} of type {@code R}
         */
        private Promise<R> getPromise() {
            return promise;
        }
    }

    /**
     * The executor function that is passed to the {@link Promise#Promise(Executor)} constructor.
     *
     * @param <T> The type with which the {@code Promise} will be fulfilled
     * @see Promise#Promise(Executor)
     * @see Promise.Action
     */
    public interface Executor<T> {
        void execute(Action<T> action);
    }

    /**
     * The callback function that is passed to callback methods of {@code Promise}.
     *
     * <p>A {@code Promise.Callback} must be allowed to return either a value of type {@code R}
     * or a new {@code Promise} of type {@code R}. However, since Java does not allow
     * return types to be this ambiguous (without the need for casting), {@code Promise.Callback}
     * must wrap the return value in a {@code Promise.Result} (unless it does not want to return
     * anything, in which case {@code null} must be returned).</p>
     *
     * @param <T> The type with which the current {@code Promise} will be fulfilled
     * @param <R> The return type with which the earlier returned {@code Promise} will be fulfilled
     * @see Promise#andThen(Callback, Callback)
     * @see Promise#andThen(Callback)
     * @see Promise#andCatch(Callback)
     * @see Promise#andFinally(Callback)
     */
    public interface Callback<T, R> {
        Result<R> execute(T value);
    }

    /**
     * A container object that holds the {@link Promise.Callback} functions for asynchronous
     * processing, after the {@code Promise} has been fulfilled.
     *
     * @param <T> The type with which the current {@code Promise} will be fulfilled
     * @param <R> The return type with which the returned {@code Promise} will be fulfilled
     * @see Promise#addCallbacks(Callback, Callback)
     * @see Promise#andThen(Callback, Callback)
     * @see Promise#andThen(Callback)
     * @see Promise#andCatch(Callback)
     * @see Promise#andFinally(Callback)
     */
    private final static class CallbackContainer<T, R> {

        private final Callback<T, R> onFulfilled;

        private final Callback<Throwable, R> onRejected;

        public CallbackContainer(final Callback<T, R> onFulfilled, final Callback<Throwable, R> onRejected) {
            this.onFulfilled = onFulfilled;
            this.onRejected = onRejected;
        }
    }

    /**
     * The possible states of a {@code Promise}.
     */
    private enum State {
        PENDING,
        FULFILLED,
        REJECTED
    }

    /**
     * The list of {@link Promise.CallbackContainer} objects that are held while the {@code Promise}
     * is still in a {@link Promise.State#PENDING} state.
     */
    private final List<CallbackContainer<T, ?>> callbackContainers = new ArrayList<>();

    /**
     * The {@link Promise.Action} that is associated with this {@code Promise}.
     */
    private final Action<T> action = new Action<>(this);

    /**
     * The {@link Promise.Executor} that was passed to this {@code Promise}.
     */
    private final Executor<T> executor;

    /**
     * Holds the current {@link State} of this {@code Promise}.
     */
    private State state = State.PENDING;

    /**
     * The {@link Handler} uses to add callbacks asynchronously.
     *
     * @see Promise#getHandler()
     * @see Promise#addCallbacks(Callback, Callback)
     */
    private Handler handler;

    /**
     * The value of type {@code T} with which this Promise might be fulfilled.
     */
    private T value;

    /**
     * The {@code Throwable} with which this Promise might be rejected.
     */
    private Throwable error;

    /**
     * Returns a {@code Promise<T>} that is fulfilled with {@code T value}.
     *
     * @param value the value of type {@code T} with which the returned {@code Promise} will be fulfilled
     * @param <T> the type of the returned {@code Promise}
     * @return the {@code Promise<T>}
     */
    public static <T> Promise<T> resolve(final T value) {
        return new Promise<>(new Executor<T>() {
            @Override
            public void execute(final Action<T> action) {
                action.resolve(value);
            }
        });
    }

    /**
     * Identity function that returns the {@code Promise<T>} argument.
     *
     * @param value the {@code Promise} of type {@code T} which will be returned
     * @param <T> the type of the returned {@code Promise}
     * @return the {@code Promise<T>}
     */
    public static <T> Promise<T> resolve(final Promise<T> value) {
        return value;
    }

    /**
     * Returns a {@code Promise<T>} that is rejected with {@code Throwable value}.
     *
     * @param value the {@code Throwable} with which the returned {@code Promise} will be rejected
     * @param <T> the type of the returned {@code Promise}
     * @return the {@code Promise<T>}
     */
    public static <T> Promise<T> reject(final Throwable value) {
        return new Promise<>(new Executor<T>() {
            @Override
            public void execute(final Action<T> action) {
                action.reject(value);
            }
        });
    }

    /**
     * Constructs a {@code Promise} with the {@link Promise.Executor} function it should execute.
     *
     * @param executor the {@link Promise.Executor} function this {@code Promise} should execute
     */
    @SuppressWarnings("ConstantConditions")
    public Promise(@NonNull Executor<T> executor) {
        if(executor == null) {
            throw new NullPointerException("Constructor argument 'Executor<T> executor' must not be null.");
        }

        this.executor = executor;

        execute();
    }

    /**
     * The callback method for adding {@link Promise.Callback}s that will be called when this
     * {@code Promise} is resolved.
     *
     * @param onFulfilled the {@link Promise.Callback} to execute when this {@code Promise} is fulfilled
     * @param onRejected the {@link Promise.Callback} to execute when this {@code Promise} is rejected
     * @param <R> the return type of the {@link Promise.Callback} arguments and the type of the {@code Promise} that is returned by this method
     * @return a new {@code Promise} that will eventually be resolved
     */
    public <R> Promise<R> andThen(final Callback<T, R> onFulfilled, final Callback<Throwable, R> onRejected) {
        return new Promise<>(new Executor<R>() {
            @Override
            public void execute(final Action<R> action) {
                addCallbacks(
                    new Callback<T, R>() {
                        @Override
                        public Result<R> execute(final T value) {
                            if(onFulfilled != null) {
                                try {
                                    action.resolve(onFulfilled.execute(value));
                                }
                                catch(Throwable e) {
                                    action.reject(e);
                                }
                            }
                            else {
                                action.resolve((R) null);
                            }

                            return null;
                        }
                    },
                    new Callback<Throwable, R>() {
                        @Override
                        public Result<R> execute(final Throwable value) {
                            if(onRejected != null) {
                                try {
                                    action.resolve(onRejected.execute(value));
                                }
                                catch(Throwable e) {
                                    action.reject(e);
                                }
                            }
                            else {
                                action.reject(value);
                            }

                            return null;
                        }
                    }
                );
            }
        });
    }

    /**
     * The callback method for adding a {@link Promise.Callback} that will be called when this
     * {@code Promise} is fulfilled.
     *
     * @param onFulfilled the {@link Promise.Callback} to execute when this {@code Promise} is fulfilled
     * @param <R> the return type of the {@link Promise.Callback} argument and the type of the {@code Promise} that is returned by this method
     * @return a new {@code Promise} that will eventually be resolved
     */
    public <R> Promise<R> andThen(final Callback<T, R> onFulfilled) {
        return andThen(onFulfilled, null);
    }

    /**
     * The callback method for adding a {@link Promise.Callback} that will be called when this
     * {@code Promise} is rejected.
     *
     * @param onRejected the {@link Promise.Callback} to execute when this {@code Promise} is rejected
     * @param <R> the return type of the {@link Promise.Callback} argument and the type of the {@code Promise} that is returned by this method
     * @return a new {@code Promise} that will eventually be resolved
     */
    public <R> Promise<R> andCatch(final Callback<Throwable, R> onRejected) {
        return andThen(null, onRejected);
    }

    /**
     * The callback method for adding a {@link Promise.Callback} that will <em>always</em> be called when this
     * {@code Promise} is resolved (as either fulfilled or rejected).
     *
     * @param onFinally the {@link Promise.Callback} to execute when this {@code Promise} is resolved
     * @param <R> the return type of the {@link Promise.Callback} argument and the type of the {@code Promise} that is returned by this method
     * @return a new {@code Promise} that will eventually be resolved
     */

    public <R> Promise<R> andFinally(final Callback<Void, R> onFinally) {
        Callback<T, R> onResolvedFinallyWrapper = new Callback<T, R>() {
            @Override
            public Result<R> execute(final Object value) {
                return onFinally.execute(null);
            }
        };

        Callback<Throwable, R> onRejectedFinallyWrapper = new Callback<Throwable, R>() {
            @Override
            public Result<R> execute(final Throwable value) {
                return onFinally.execute(null);
            }
        };

        return andThen(onResolvedFinallyWrapper, onRejectedFinallyWrapper);
    }

    /**
     * Execute the {@link Promise#executor} function, by passing it {@link Promise#action} and
     * possibly rejecting this {@code Promise} with any {@code Throwable} it throws.
     */
    private void execute() {
        try {
            executor.execute(action);
        }
        catch(Throwable e) {
            rejectInternal(e);
        }
    }

    /**
     * Fulfills this {@code Promise} with the value of type {@code T}, <em>unless</em> this {@code Promise}
     * was already previously resolved.
     *
     * @param fulfillValue the final value with which this {@code Promise} should be fulfilled
     */
    private void fulfillInternal(final T fulfillValue) {
        synchronized(callbackContainers) {
            if(state == State.PENDING) {
                state = State.FULFILLED;
                value = fulfillValue;
                handleCallbacks();
                callbackContainers.clear();
            }
        }
    }

    /**
     * Rejects this {@code Promise} with a {@code Throwable}, <em>unless</em> this {@code Promise}
     * was already previously resolved.
     *
     * @param rejectValue the {@code Throwable} with which this {@code Promise} should be rejected
     * @see Promise.Action#reject(Throwable)
     */
    private void rejectInternal(final Throwable rejectValue) {
        synchronized(callbackContainers) {
            if(state == State.PENDING) {
                state = State.REJECTED;
                error = rejectValue;
                handleCallbacks();
                callbackContainers.clear();
            }
        }
    }

    /**
     * Resolves this {@code Promise} with a {@link Promise.Result}, <em>unless</em> this {@code Promise}
     * was already previously resolved.
     *
     * @param result the {@code Promise.Result} with which this {@code Promise} should be resolved
     * @see Promise.Action#resolve(T)
     * @see Promise.Action#resolve(Result)
     * @see Promise.Action#resolve(Promise)
     * @see Promise#fulfillInternal(T)
     */
    private void resolveInternal(final Result<T> result) {
        if(result == null) {
            fulfillInternal(null);
        }
        else if(result.getType() == Result.Type.VALUE) {
            fulfillInternal(result.getValue());
        }
        else {
            result.getPromise().andThen(
                new Callback<T, T>() {
                    @Override
                    public Result<T> execute(final T value) {
                        action.resolve(new Result<>(value));

                        return null;
                    }
                },
                new Callback<Throwable, T>() {
                    @Override
                    public Result<T> execute(final Throwable value) {
                        action.reject(value);

                        return null;
                    }
                }
            );
        }
    }

    /**
     * The {@link Handler} used by {@link Promise#addCallbacks} to add them at the
     * back of the message queue of {@link Looper#getMainLooper}, so they will be
     * processed asynchronously.
     *
     * @return Handler
     * @see Promise#addCallbacks(Callback, Callback)
     */
    private Handler getHandler() {
        if(handler == null) {
            handler = new Handler(Looper.getMainLooper());
        }

        return handler;
    }

    /**
     * Process a single {@link Promise.CallbackContainer}, either by adding them
     * to the {@link Promise#callbackContainers} list if this {@code Promise} is still
     * {@link Promise.State#PENDING} or executing one of its {@link Promise.Callback}s
     * immediately, depending on the resolved {@link Promise#state}.
     *
     * @param callbackContainer the {@link Promise.CallbackContainer} that needs processing
     * @param <R> the return type with which the earlier returned {@code Promise} will be fulfilled
     */
    private <R> void handleCallbacks(final CallbackContainer<T, R> callbackContainer) {
        synchronized(callbackContainers) {
            if(state == State.PENDING) {
                callbackContainers.add(callbackContainer);
            }
            else if(state == State.FULFILLED && callbackContainer.onFulfilled != null) {
                callbackContainer.onFulfilled.execute(value);
            }
            else if(state == State.REJECTED && callbackContainer.onRejected != null) {
                callbackContainer.onRejected.execute(error);
            }
        }
    }

    /**
     * Process all pending {@link Promise.CallbackContainer}s.
     *
     * @see Promise#fulfillInternal(T)
     * @see Promise#rejectInternal(Throwable)
     * @see Promise#handleCallbacks(CallbackContainer)
     */
    private void handleCallbacks() {
        synchronized(callbackContainers) {
            for(CallbackContainer<T, ?> callbackContainer : callbackContainers) {
                handleCallbacks(callbackContainer);
            }
        }
    }


    /**
     * Adds a {@link Promise.CallbackContainer} to {@link Promise#callbackContainers} asynchronously,
     * by posting it to back of the message queue of {@link Handler}.
     *
     * @param onFulfilled the {@link Promise.Callback} to execute when this {@code Promise} is fulfilled
     * @param onRejected the {@link Promise.Callback} to execute when this {@code Promise} is rejected
     * @param <R> the return types of the {@link Promise.Callback} arguments and the earlier returned {@code Promise}
     * @see Promise#andThen(Callback, Callback)
     * @see Promise#andThen(Callback)
     * @see Promise#andCatch(Callback)
     * @see Promise#andFinally(Callback)
     */
    private <R> void addCallbacks(final Callback<T, R> onFulfilled, final Callback<Throwable, R> onRejected) {
        final CallbackContainer<T, R> callbackContainer = new CallbackContainer<>(onFulfilled, onRejected);

        // force async execution
        getHandler().postDelayed(new Runnable() {
            @Override
            public void run() {
                handleCallbacks(callbackContainer);
            }
        }, 0);
    }
}

Example usage

new Promise<>(new Promise.Executor<Integer>() {
    @Override
    // make action final for accessibility inside Runnable
    public void execute(final Promise.Action<Integer> action) {
        // simulate some fictional asynchronous routine
        new Thread(new Runnable() {
            @Override
            public void run() {
                try {
                    // sleep for 5 seconds
                    Thread.sleep(5000);
                    // resolve with integer 1
                    action.resolve(1);
                }
                catch(Throwable e) {
                    // or reject with some caught Throwable
                    action.reject(e);
                }
            }
        }).start();
    }
})
// accepts the Integer result and returns a new Promise<Boolean>
.andThen(new Promise.Callback<Integer, Boolean>() {
    @Override
    public Promise.Result<Boolean> execute(Integer value) {
        System.out.println("This is executed when the previous promise is fulfilled. Got: " + value);

        // Boolean must be wrapped in a Promise.Result<Boolean>, because Promise.Callback<..., Boolean>
        // must be allowed to return another Promise<Boolean> as well
        return new Promise.Result<>(false);
    }
})
// accepts the Boolean result and returns a new Promise<Void>
.andThen(new Promise.Callback<Boolean, Void>() {
    @Override
    public Promise.Result<Void> execute(Boolean value) {
        System.out.println("This is executed when the previous promise is fulfilled. Got: " + value);
        return null;
    }
})
// catches any uncaught Throwable up the chain and returns a new Promise<Void>
.andCatch(new Promise.Callback<Throwable, Void>() {
    @Override
    public Promise.Result<Void> execute(Throwable value) {
        System.out.println("This is executed when a promise up the chain is rejected.");
        return null;
    }
})
// will always be called, regardless of the settled state of promises up the chain
// and returns a new Promise<Void>
.andFinally(new Promise.Callback<Void, Void>() {
    @Override
    public Promise.Result<Void> execute(Void value) {
        System.out.println("This will always be executed.");
        return null;
    }
});
\$\endgroup\$
6
  • 1
    \$\begingroup\$ Not an answer to your question, but have you looked at the class java.util.concurrent.CompletableFuture? What you do there looks very much like CompletableFuture.supplyAsync(...).thenAcceptAsync(...) to me. (Thus re-inventing the wheel.) \$\endgroup\$
    – mtj
    May 12 '20 at 12:05
  • \$\begingroup\$ Hi @mtj. Yes, I was aware of that class. However, I find it too bloated for my current needs. And, more importantly, it is only available from Android API 24 onward, while I'm still coding for Android API 19. Cheers, though. \$\endgroup\$
    – Codifier
    May 12 '20 at 15:31
  • \$\begingroup\$ This question is of course fine for CodeReview, but generally I'd rather copy a "bloated" but well tested implementation than reinventing my own, especially when it comes to thread-safety. Note to other readers: relies on Android specific classes and annotations! \$\endgroup\$ May 12 '20 at 15:52
  • \$\begingroup\$ @MaartenBodewes Fair enough, perhaps I should have explicitly stated in the question that it's meant for Android API 19 onward, so CompletableFuture is not available. About your note though: I mean, it's clearly stated in the title, the introduction and implied in the tags already that this implementation is meant for Android. \$\endgroup\$
    – Codifier
    May 12 '20 at 16:29
  • \$\begingroup\$ Yeah, I know, but just to get started quickly I tried compiling in my Java environment, which of course did not work. Usually I can get it to work pretty easily - since it is a rather generic component - but without the Handler code it's just not that easy. \$\endgroup\$ May 12 '20 at 17:02
3
+100
\$\begingroup\$

I'm not familiar with Promise from JavaScript and this is quite a complex flow, so hopefully I didn't miss much.

Let's just review the flow a bit

new Promise<>(new Promise.Executor<Integer>() {
    @Override
    public void execute(Promise.Action<Integer> action) {
        // Perhaps do something asynchronous and then:
        action.resolve(1);
    }
})

In this case, the entire execution is done within you constructor, under execute(). There is access to some properties in the class, however it is done in a single thread. So no problems so far.

.andThen(new Promise.Callback<Integer, Boolean>() {
    @Override
    public Promise.Result<Boolean> execute(Integer value) {
        return new Promise.Result<>(false);
    }
})

Now things start to complicate a bit. andThen creates a new Promise which adds this callback to the original Promise. The callback it self, is transferred to an Handler, and executed using postDelayed.

We know for certain that the first execution of the original Promise was done, since it is in the constructor. So there is no race with it. We also know that the callback doesn't access any of the properties in our Promise. So far it looks safe. Let's take a look at the execution of the callbacks:

    private <R> void handleCallbacks(final CallbackContainer<T, R> callbackContainer) {
        synchronized(callbackContainers) {
            if(state == State.PENDING) {
                callbackContainers.add(callbackContainer);
            }
            else if(state == State.FULFILLED && callbackContainer.onFulfilled != null) {
                callbackContainer.onFulfilled.execute(value);
            }
            else if(state == State.REJECTED && callbackContainer.onRejected != null) {
                callbackContainer.onRejected.execute(error);
            }
        }
    }

The first thing done in this method is entering a synchronized block, so any modifications to our states are going to be safe. This includes modifications to state and callbackContainers.

Let's say we have 2 callbacks. One by one, the call is done to Handler.postDelayed, with execution of those callbacks. We know they are added to postDelayed in order, so they should be executed in order (I'm not sure 100% that postDelayed guarantees that`.

Let's dive in a little. Say callbackContainer.onFulfilled.execute(value); is called, This will invoke this code section:

new Promise.Callback<T, R>() {
        @Override
        public Promise.Result<R> execute(final T value) {
            if(onFulfilled != null) {
                try {
                    action.resolve(onFulfilled.execute(value));
                }
                catch(Throwable e) {
                    action.reject(e);
                }
            }
            else {
                action.resolve((R) null);
            }

            return null;
        }
    }

Let's say a value is returned from onFulfilled, and action.resolve(onFulfilled.execute(value)); is called. This calls resolveInternal, and called lead to a call to fulfillInternal.

    private void fulfillInternal(final T fulfillValue) {
        synchronized(callbackContainers) {
            if(state == State.PENDING) {
                state = State.FULFILLED;
                value = fulfillValue;
                handleCallbacks();
                callbackContainers.clear();
            }
        }
    }

Luckily state is fulfilled already, so this won't be executed. What if it isn't? The modification of values is done in a synchronized block, so it is safe. How about handleCallbacks? Well it will run all callbacks inside callbackContainer, and because the callback use the same lock, and are only added to the container from postDelayed under that lock, it shouldn't be a problem.

Although the code is complex, and I have talked here a lot, it is actually quite solid. The limitations on state are well placed, the use of the lock is done where needed, and the callbacks are executed well. I could try to dig further, but I have to go.

So well done!

A couple of other things:

  • Wayyy to much code in one file. All the inner types and the javadoc make it difficult to navigate. You should extract the inner classes.
  • I can't really find much way that it after execute in the constructor, the value won't be PENDING. To do this, I have to run an async operation from inside the Executor, and modify the action there. Which is weird. Maybe I'm missing something?
  • Exposing Action to users is quite dangerous, since it is a state of the class, and they can really abuse it.
\$\endgroup\$
5
  • \$\begingroup\$ Thank you very much for your review! It appears your conclusions about the synchronized parts coincide with my own, so that's reassuring. I'll address your three end remarks in separate comments. About your first: I definitely see where you are coming from, but for some reason, because all classes are so closely linked to the Promise and without much use outside of it, I actually like them as inner classes. But perhaps that's a bit silly and I should reevaluate this. \$\endgroup\$
    – Codifier
    May 15 '20 at 13:49
  • \$\begingroup\$ About your second: Indeed, the whole point of a Promise is to do something asynchronous in the Executor, during which the Promise remains PENDING, and after which you either resolve() or reject(). For brevity I haven't given a concrete example of this, but could surely add an asynchronous example, if necessary. Furthermore, another thing about Promises is, is that even after a Promise is settled (as either fulfilled or rejected) and thus not PENDING anymore, one must still be able to add callbacks to them, that will then still be executed, depending on the settled state. \$\endgroup\$
    – Codifier
    May 15 '20 at 13:51
  • \$\begingroup\$ Finally, about your third: In JavaScript Promises the resolve() and reject() functions are passed as arguments to the executor, but I didn't see a neat way to do this for a typed Promise in Java, particularly because I need overloaded versions of resolve(), to allow for either a concrete value of the projected type, or another Promise that resolves to that type, so I pass an Action object. Furthermore, the Action class is final and has a private constructor, so I don't see any immediate vectors of abuse. Or am I missing something? \$\endgroup\$
    – Codifier
    May 15 '20 at 13:53
  • \$\begingroup\$ I took a look at the flow of using Action in a delay, and again it seems the synchronized blocks are keeping the flows proper. My comment about exposing Action is more of a general safety tip. In your case exposing Action doesn't seem to be a danger. I will say that abusing is always possible, through reflection for instance, but stopping that is a different story. \$\endgroup\$
    – tomtzook
    May 15 '20 at 22:12
  • \$\begingroup\$ Yeah, exactly: with reflection all bets are off. :-) Anyway, thanks again for looking into this. \$\endgroup\$
    – Codifier
    May 15 '20 at 23:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.