Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Promise callbacks should be deferred whenever possible #16

Open
ivan4th opened this issue Jan 12, 2015 · 3 comments
Open

Promise callbacks should be deferred whenever possible #16

ivan4th opened this issue Jan 12, 2015 · 3 comments

Comments

@ivan4th
Copy link
Contributor

ivan4th commented Jan 12, 2015

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
Copy link
Contributor Author

ivan4th commented Jan 12, 2015

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
Copy link
Contributor Author

ivan4th commented Jan 12, 2015

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
Copy link
Owner

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants