orthecreedence / blackbird

Common Lisp promise implementation (successor to cl-async-future)
83 stars 10 forks source link

Promise callbacks should be deferred whenever possible #16

Open ivan4th opened 9 years ago

ivan4th commented 9 years ago

As of now, promise error callbacks are always invoked immediately and promise success callbacks are only deferred if *promise-finish-hook* is set, e.g.:

(setf blackbird:*promise-finish-hook*
      #'(lambda (fn)
          (as:with-delay () (funcall fn))))

That's quite unlike js promises, where both success and error callbacks are always executed only when control reaches the event loop.

The problem is that code that uses immediate callbacks often needs to be written in very different style from the code that uses deferred callbacks, and changing blackbird:*promise-finish-hook* may break some of the code depending on which style it is using.

Although immediate callback invocation is faster, it may cause some very weird code flow. E.g. you may have some code like

(defun talk-to-server (message)
  (bb:with-promise (resolve reject)
    (set-message-handler
     #'(lambda (message)
         (resolve message)
         (clear-message-handler)))
    ;; send-message returns immediately
    (send-mesage message)))

(defun dialogue ()
  (bb:alet ((response (talk-to-server message-1)))
    (talk-to-server (make-message-2-for-response response))))

Let's see what happens in dialogue. First talk-to-server call is made, which returns a promise which will be resolved sometime in future when a message arrives from the server. Until the message arrives, there's a message handler installed that will resolve the promise. Then, when the message is received, the promise is resolved and second talk-to-server call is made. It sets a new message handler (which may override the previous one, ok), initiates message sending and returns a promise. The problem is that the first (resolve message) call from the first (talk-to-server) call returns at this point, and (clear-message-handler) is called, killing the newly installed message handler. Such very strange "blast from the past" thing made me spend about 3 hours trying to debug a similar problem in my MQTT driver. Other, even more weird "unwanted reentrancy" pattens may occur, too.

Deferring error callbacks is also important because such callbacks may activate other long promise chains.

I'd suggest either making some bb-async ASDF system which can be added to either cl-async or blackbird, and/or using asdf-system-connections to set blackbird:*promise-finish-hook* to delaying function when cl-async is available.

Perhaps error callbacks should be updated to use blackbird:*promise-finish-hook*, too.

ivan4th commented 9 years ago

The example may seem to be a bit dumb, in fact it was a bit more involved with a list of handlers being cleared after every handler is run.

Also, it may be worth adding something like :immediate t for the cases when one knows what he/she is doing, like resolving the promise at the very end of some async callback which is guaranteed to be called asynchronously.

ivan4th commented 9 years ago

Perhaps bb-async could also contain some useful wrappers for cl-async functionality, such as delay-promise (promise-based replacement for as:delay), spawn-wait (spawn a process and wait for its termination), mkdtemp-promise and so on.

orthecreedence commented 9 years ago

Sorry it took me so long to respond, this has been a very busy month for me (outside of lisp stuff).

I'm fine with introducing a change in blackbird that defers callbacks whenever possible (including error callbacks). The first thing to change is to update *promise-finish-hook* to work for error handling, I think.

I think it would be difficult to force changing the hook depending on the package, but what if the default finish hook itself was able to detect whether it was in an event loop?

(lambda (finish-fn)
  (if (and (find-package :cl-async)
           (symbol-value (intern '*event-base* :cl-async)))
      (as:delay finish-fn)
      (funcall finish-fn)))

Thoughts?