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

A race condition between schedule-render! and send cb's reconcile! that prevents some future reconciling #862

Open
bnoguchi opened this issue Apr 5, 2017 · 0 comments

Comments

@bnoguchi
Copy link
Contributor

bnoguchi commented Apr 5, 2017

I discovered a race condition that prevents future reconciling from occurring. I've submitted a PR GH-863 with a devcards test and a fix.

Explanation

Right now, this block of code can result in a race condition that can stop future reconciling from occurring. Block of code inlined next for convenience:

            ([resp query remote]
             (when-not (nil? remote)
               (p/queue! this (keys resp) remote))
             (merge! this resp query remote)
             (p/reconcile! this remote))))))))

In particular, (merge! ...) will cause a state change that can trigger a (schedule-render! ...) and therefore eventually a reconcile. In cases where the requestAnimationFrame immediately runs the scheduled reconcile, "eventually" becomes "immediately." During this immediate reconcile, the following line will set :queued to false:

      (swap! state update-in [:queued] not)

In this scenario, the last(p/reconcile! this remote) in the code block above will run right after the reconcile triggered by requestAnimationFrame. This reconcile will also call the following line:

      (swap! state update-in [:queued] not)

which will set :queued back to true, without adding enqueueing anything to :queue-remote or :queue.

This puts our reconciler into a state in which subsequent calls to schedule-render! will think that there is already a render scheduled and won't queue a render and reconcile.

   (defn schedule-render! [reconciler]
     (when (p/schedule-render! reconciler)
       (queue-render! #(p/reconcile! reconciler))))

where p/schedule-render! is defined as follows:

  (schedule-render! [_]
    (if-not (:queued @state)
      (do
        (swap! state assoc :queued true)
        true)
      false))

This means that any reconciles that would result from a change in our reconciler :state won't trigger a reconcile. See here:

        (add-watch (:state config) (or target guid)
          (fn [_ _ _ _]
            (swap! state update-in [:t] inc)
            #?(:cljs
               (if-not (iquery? root-class)
                 (queue-render! parsef)
                 (schedule-render! this)))))

And the symptom will be observed state changes not re-rendering our ui.

bnoguchi added a commit to bnoguchi/om that referenced this issue Apr 5, 2017
bnoguchi added a commit to bnoguchi/om that referenced this issue Apr 5, 2017
@bnoguchi bnoguchi changed the title A race condition between schedule-render! and send cb's reconcile! that preventing future reconciling A race condition between schedule-render! and send cb's reconcile! that prevents some future reconciling Apr 5, 2017
bnoguchi added a commit to bnoguchi/om that referenced this issue Jul 10, 2017
bnoguchi added a commit to bnoguchi/om that referenced this issue Jul 10, 2017
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

1 participant