Closed DanFu09 closed 9 years ago
the only way this could come up, would be if we were in an environment which redefined setTimeout in terms of process.nextTick which meant that the clean up function we thought would only run if the loop exited prematurely actually ran during the look because setTimeout was actually an alias for process.nextTick.
do Babel, Reflux, or React happen to set their own versions of setTimeout? if So why doesn't this usage cause a problem, the only difference is that it has the 0 explicitly stated, I wonder if changing this line to be
var timeout = setTimeout(cleanUpNextTick, 0);
or even
var timeout = setTimeout(cleanUpNextTick, 4);
(which should be equivalent in a browser) would fix it.
I'm seeing a similar problem in an environment that I'm putting together (ottoext) on top of the otto JavaScript interpreter. That interpreter, as with many others, doesn't contain an event loop. The way that I've implemented setTimeout
and friends means that setTimeout(fn)
is exactly the same as process.nextTick(fn)
.
There's a crash in cleanUpNextTick
that has its roots in the same place as this one, which can also be papered over by changing setTimeout(cleanUpNextTick)
to setTimeout(cleanUpNextTick, 5)
. Inside cleanUpNextTick
, currentQueue
can be null
sometimes. I presume it's by the same mechanism as described above.
Perhaps a null check would be good in cleanUpNextTick
as well? Either way, I'm going to find out what the correct behaviour is for setTimeout
and make sure I implement that, but it could save future JavaScript environment tinkerers some time tracking down mysterious crashes.
Omitting the number from setTimeout should be equivalent to setTimeout(fun, 4)
On Sat, Oct 3, 2015, 11:41 PM Conrad Pankoff notifications@github.com wrote:
I'm seeing a similar problem in an environment that I'm putting together ( ottoext http://fknsrs.biz/p/ottoext) on top of the otto https://github.com/robertkrimen/otto JavaScript interpreter. That interpreter, as with many others, doesn't contain an event loop. The way that I've implemented setTimeout and friends means that setTimeout(fn) is exactly the same as process.nextTick(fn).
There's a crash in cleanUpNextTick that has its roots in the same place as this one, which can also be papered over by changing setTimeout(cleanUpNextTick) to setTimeout(cleanUpNextTick, 5). Inside cleanUpNextTick, currentQueue can be null sometimes. I presume it's by the same mechanism as described above.
Perhaps a null check would be good in cleanUpNextTick as well? Either way, I'm going to find out what the correct behaviour is for setTimeout and make sure I implement that, but it could save future JavaScript environment tinkerers some time tracking down mysterious crashes.
— Reply to this email directly or view it on GitHub https://github.com/defunctzombie/node-process/issues/49#issuecomment-145315304 .
Yep, that's the behaviour I ended up implementing. I still had very, very occasional crashes at the same point, but they were certainly less frequent. I ended up managing to avoid the problem entirely by implementing setImmediate
on my own and telling webpack (which was pulling this in, indirectly) not to shim the process
module.
If currentQueue is null in cleanUpNextTick that would likely be a seperate thing that would be worth opening it's own issue for. Off the top of my head it could be caused by clearTimeout not effectively removing the timeouts?
I'm getting an error in Firefox where
currentQueue
isnull
inside ofdrainQueue
inbabel-core/browser.js
. From stepping through the code, there appears to be a race condition between two instances ofdrainQueue
running at once. This is what is happening:drainQueue
starts the first time:queueIndex
is -1,queue
has one item in itdraining
is set to truesetTimeout(cleanUpNextTick)
calledcurrentQueue
is set toqueue
,queue
is set to an empty arrayqueueIndex
compared to length ofqueue
and then incrementedcurrentQueue[queueIndex].run()
calledcleanUpNextTick
scheduled bydrainQueue
draining
set to falsecleanUpNextTick
setsqueue
tocurrentQueue
and callsdrainQueue
, starting a second instance ofdrainQueue
currentQueue
set to aqueue
(which is just what it was beforequeueIndex
is already1
, no new functions are runqueueIndex
set to -1currentQueue
is set to nulldrainQueue
completesdrainQueue
regains controlqueueIndex
is -1, because of seconddrainQueue
queueIndex
is less thanlen
, socurrentQueue[queueIndex].run()
is calledcurrentQueue
is nullI've only observed this problem in Firefox, and usually only under very specific conditions (usually only the first time the page is loaded). We think this is causing problems for us downstream with Babel, Reflux, and React, so a fix would be greatly appreciated!