The minimongo calls that cared about waiting for observe callbacks to be
fired before returning use drain() on a Meteor._SynchronousQueue to do
that wait. But if drain is called from within a task on the same queue,
drain returned immediately, which failed to provide the desired
This commit changes drain in this context to throw instead. This would
now make a reentrant observeChanges (or mutator) throw, so we fix that
by splitting up LocalCollection._observeQueue into two different pieces.
One piece is used purely for the above "drain" purpose: this is the
stack of queues LocalCollection._observeQueues. Each call to
observeChanges/insert/update/remove/resumeObservers gets a new queue
which it can drain, and since it's a new queue, the drain call is not
within a task on the *same queue*, so the error case doesn't occur.
However, just doing this would break the constraint that multiple
observe callbacks for the same observe are always called in
sequence (even if on the server, one yields). So we use a *separate*
queue, which is per-observe (and which never gets drained) for this
purpose. (This ensures that the test "mongo-livedata - minimongo observe
on server" still passes.)
Note that #2275 was made more obvious by df2820f, which caused the
common case of findOne inside a helper inside a `#each` to end up
dependent on the synchronous observeChanges contract. Reverting that
commit does fix our newly added test "minimongo - fetch in observe" but
it does not help "minimongo - observe in observe".
except that this leads to the unacceptable behavior where the
newly-added-in-this-commit test "minimongo - insert in observe"
That's because fundamentally, the following things cannot all be
(a) For a given observeChanges calls, its callbacks will be invoked in
sequence; they cannot overlap either due to server-side yielding or due
(b) When LocalCollection.insert() is called, all relevant observe
callbacks are invoked before it returns.
(c) It is legal to call insert (or another mutator) on a collection from
within an observe callback in such a way that it will affect the result
of the observed cursor.
This commit resolves this issue by making (c) illegal. But this is
probably a pretty valid use case. So we are not actually planning to
merge this commit.
Instead, we should probably relax (b): we should probably not drain the
queue anywhere but in observeChanges. This would actually be consistent
with how it works on the server, where you have to use a write fence if
you'd like to wait for all observes to be called. This is a bigger
change than we have time for now, because we'd have to ensure that write
fences work on the client, make minimongo interact with them, and
rewrite all of our minimongo tests that care about observe timing to use
write fences (just like the mongo-livedata tests already do). So for now
we will probably "fix" #2275 in a more ad-hoc and targeted way.
Make some instances of #2315 into errors instead of silent early
Specifically, observeChanges calls (with added or addedBefore callbacks)
from within another observeChanges callback on the same collection will
be unable to differentiate between initial and later added/addedBefore
calls, which is serious enough to be an error (see #2315 for details).
We don't currently think that the other effect of #2315 (where observe
callbacks triggered by insert/remove/update/resumeObservers will not
occur before those methods return, if they are called reentrantly) is
problematic enough to deserve this sort of error.
This reverts commit c05ae24 EXCEPT for
the "fetch in observe" test, which I'm leaving in because we still want
a test for #2275.
This commit made it an error to start an observeChanges from within an
observeChanges callback for the same collection, but it turns out that
this is actually a somewhat common thing to do (for example, nested
'each'). Instead, we'll leave things the way they were pre-0.8.2: you
can start an observeChanges from within an observeChanges callback, but
it'll be subtly buggy in that you won't get synchronous 'added'
callbacks. This issue is described in #2315, along with the fact that
insert/update/remove/resumeObservers won't run their affected observe
callbacks if they are called from within a task on the collection's
queue. Eventually we'll implement the full fix (which relaxes the
requirement that insert/update/remove run all their callbacks before
returning) described in #2315.