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

setImmediate and MessageChannel broken on IE10 #197

Closed
plaa opened this issue Sep 21, 2013 · 25 comments
Closed

setImmediate and MessageChannel broken on IE10 #197

plaa opened this issue Sep 21, 2013 · 25 comments
Assignees
Milestone

Comments

@plaa
Copy link

plaa commented Sep 21, 2013

There's a bug in IE10 that makes setImmediate and MessageChannel practically useless on IE10 Mobile, and somewhat unreliable on IE10 Desktop. Simply adding a spin.js spinner on our login page caused Q promises to stop working on IE10 Mobile.

I wrote a detailed description of the bug in my blog: http://codeforhire.com/2013/09/21/setimmediate-and-messagechannel-broken-on-internet-explorer-10/

As a workaround, we had to disable both setImmediate and MessageChannel in order to get Q promises to work:
window.setImmediate = undefined;
window.MessageChannel = undefined;

when.js seems to use setImmediate if present, so it's affected as well. See kriskowal/q#396

@unscriptable
Copy link
Member

It'd be nice if we could detect the bug rather than sniff the browser, but it seems unlikely that we'll be able to do this, unfortunately. Regardless, we should add a work-around in poly.js.

Not sure what to do about when.js.

@unscriptable
Copy link
Member

Not sure what to do about when.js since MessageChannel is also affected. @plaa, do you have a link to the bug you filed with Microsoft?

@stefanpenner
Copy link

@briancavalier
Copy link
Member

Wow, just wow. So setIMMEDIATE is a complete lie :) Thanks for the report and very detailed blog post, @plaa! Thanks for the links to the related issues, @plaa and @stefanpenner.

I'm sure at some point MS will fix this, but who knows how long that will take to propagate to Win 8 mobiles in the wild, so yeah, we need to do something relatively quickly.

One thought is that we could drop support for setImmediate entirely. That'd cause when.js to use process.nextTick on node, which for us is fine, since we manage our own queue and would never call process.nextTick recursively (which is the purpose of setImmediate on node, as I understand it). Supposedly process.nextTick is also slightly faster.

That leaves us with what to do on IE10. Once we drop setImmediate, it would fall back to MessageChannel, which has the same problem. Too bad IE10 doesn't support MutationObserver, or that would be a great solution (apparently, IE11 does). We may want to do this anyway based on some recent research done by @calvinmetcalf (blog post).

Perhaps postMessage would be a decent solution? What other async hacks are available in IE10?

Falling all the way back to setTimeout is an option, since we already process the handler queue in a single "tick". This would impact time-to-first-handler in IE10, but after that there'd be no additional impact. It's not ideal, but workable.

Here's a potential plan:

  1. Remove setImmediate support entirely
  2. Investigate postMessage on IE10
    • If postMessage is workable and doesn't impact other platforms, use it.
    • If not, oh well, we just let IE10 use setTimeout.
  3. Implement MutationObserver support

What do you guys think? Other alternatives?

briancavalier added a commit that referenced this issue Sep 23, 2013
…eChannel in favor of MutationObserver. This forces falling back to setTimeout in IE < 11, and Safari < 6 (see https://developer.mozilla.org/en-US/docs/Web/API/MutationObserver#Browser_compatibility).
briancavalier added a commit that referenced this issue Sep 23, 2013
…eChannel in favor of MutationObserver. This forces falling back to setTimeout in IE < 11, and Safari < 6 (see https://developer.mozilla.org/en-US/docs/Web/API/MutationObserver#Browser_compatibility).
@unscriptable
Copy link
Member

So what's the fallback for IE(6-)8? Should we look at <script>
onreadystatechange? Or is setTimeout good enough?

On Sat, Sep 21, 2013 at 6:50 PM, Brian Cavalier notifications@github.comwrote:

Wow, just wow. So setIMMEDIATE is a complete lie :) Thanks for the report
and very detailed blog post, @plaa https://github.com/plaa! Thanks for
the links to the related issues, @plaa https://github.com/plaa and
@stefanpenner https://github.com/stefanpenner.

I'm sure at some point MS will fix this, but who knows how long that will
take to propagate to Win 8 mobiles in the wild, so yeah, we need to do
something relatively quickly.

One thought is that we could drop support for setImmediate entirely.
That'd cause when.js to use process.nextTick on node, which for us is
fine, since we manage our own queue and would never call process.nextTickrecursively (which is the purpose of
setImmediate on node, as I understand it). Supposedly process.nextTick is
also slightly faster.

That leaves us with what to do on IE10. Once we drop setImmediate, it
would fall back to MessageChannel, which has the same problem. Too bad
IE10 doesn't support MutationObserver, or that would be a great solution (apparently,
IE11 doeshttps://developer.mozilla.org/en-US/docs/Web/API/MutationObserver).
We may want to do this anyway based on some recent research done by
@calvinmetcalf https://github.com/calvinmetcalf (blog posthttp://calvinmetcalf.com/post/61761231881/javascript-schedulers
).

Perhaps postMessage would be a decent solution? What other async hacks
are available in IE10?

Falling all the way back to setTimeout is an option, since we already
process the handler queue in a single "tick". This would impact
time-to-first-handler in IE10, but after that there'd be no additional
impact. It's not ideal, but workable.

Here's a potential plan:

  1. Remove setImmediate support entirely
  2. Investigate postMessage on IE10
    • If postMessage is workable and doesn't impact other platforms,
      use it.
    • If not, oh well, we just let IE10 use setTimeout.
      1. Implement MutationObserver support

What do you guys think? Other alternatives?


Reply to this email directly or view it on GitHubhttps://github.com//issues/197#issuecomment-24874345
.

@unscriptable
Copy link
Member

Hm.... this feels like it's going to turn into a game of whack-a-mole. Until JavaScript has a formal scheduler, this could get crazy. I wonder if browsers are always going to exhibit unreliable behavior under various high-cpu and/or low-memory situations.

Maybe we should just leave when.js at something simple, say setTimeout and setImmediate, and leave the setImmediate shims do the whack-a-mole?

@briancavalier
Copy link
Member

So what's the fallback for IE(6-)8?

Yeah, it's setTimeout in IE < 10, and #198 doesn't change that. It does cause IE10 to fall back to setTimeout.

Should we look at <script> onreadystatechange
Until JavaScript has a formal scheduler, this could get crazy

Yeah, it's definitely an arms race + whack-a-mole, and that sucks. As of #198, ignoring vert.x for a minute, we'd support 2 "fast" scheduler options for "common" platforms: process.nextTick for node, and MutationObserver for browsers. We'd fall back to setTimeout for anything that doesn't support either of those.

I'd rather not deal with onreadystatechange, but I guess it's an option, along with postMessage.

leave the setImmediate shims do the whack-a-mole?

That's an option. If we went that route, I think we'd need to support:

  1. process.nextTick for pre-setImmediate node (v0.8, which is still common).
  2. setImmediate node >= 0.10, and recommend a browser shim
  3. vert.x 1.x and 2.x
  4. setTimeout Ringo, plus fallback for everything except vert.x

That's mostly the situation we had before we introduced MessageChannel support. Of course, any shim we recommend would need to fix this IE10 setImmediate problem. Hmmm, does one exist currently, or is updating poly/setImmediate our best option?

@calvinmetcalf
Copy link

my fork of setImmediate is up to date (includes mutation, dosn't include setImmediate) and is an AMD module

@briancavalier
Copy link
Member

@calvinmetcalf Nice, thanks for the update.

@briancavalier
Copy link
Member

@calvinmetcalf Quick q: Does your fork detect/workaround the IE10 setImmediate/MessageChannel problem described in the original post?

@calvinmetcalf
Copy link

yes, it currenlty does not look for global setImmediate and prefers postMessage to MessageChannel

@briancavalier
Copy link
Member

@calvinmetcalf Ah, I see, thanks. You don't worry about actually polyfilling window.setImmediate. I kinda like that better than trying to be a polyfill.

@calvinmetcalf
Copy link

yeah more of a shim really

@plaa
Copy link
Author

plaa commented Sep 23, 2013

postMessage also suffers from the same bug: http://sampo.kapsi.fi/setImmediate/testPostMessage.html

Seems like there’s no fully functional zero-time timeout on IE10.

@briancavalier
Copy link
Member

@plaa wow, thanks. I think I'm ok with IE10 getting relegated to setTimeout-land until MS fixes the problem. The only other two I can think of are onreadystatechange and requestAnimationFrame. I have no idea how either behaves in IE10, and it's possible that IE10 could pause animation frames entirely for background tabs (some browser do, I think), which would obviously be bad!

@calvinmetcalf
Copy link

if I remember correct requestAnimationFrame is similar to setTimeout

@plaa if you use setTimeout twice in that example does it still break?

@plaa
Copy link
Author

plaa commented Sep 23, 2013

setTimeout works properly: http://sampo.kapsi.fi/setImmediate/testSetTimeout.html
runSetTimeout and runSetTimeout2 are called alternately

@calvinmetcalf
Copy link

postMessage works fine in IE 9, and IE11 has mutation observer, meaning if we set the use order to be muationObserver, process, setImmediate, postMessage,messageChannel,stateChange,setTimeout with setImmediate actually the same as setTimeout we'd catch just the offending browsers.

edit to be clear by works fine in IE 9 I mean works exactly as badly as the 2 setTimeouts example.

@briancavalier
Copy link
Member

works exactly as badly as the 2 setTimeouts example

ROFL!

So, does mean there's effectively no advantage at all to using postMessage in any scenario? Latest Non-IE browsers all support MutationObserver, postMessage is busted in IE10, and postMessage in IE9 performs no better than setTimeout?!?

@calvinmetcalf
Copy link

sorry by badly i meant that as the dom grew bigger in that demo it caused the browser to slow down and eventually melt down. Good or bad wasn't referring to how the promises worked, post message worked very well in the quasi real world perf example for IE9 (catiline and lie both use it as a fallback but others don't)

@plaa
Copy link
Author

plaa commented Sep 23, 2013

onreadystatechange seems to work: http://sampo.kapsi.fi/setImmediate/testOnReadyStateChange.html

On WP8 the two functions are called alternately, on IE10 desktop only runOnReadyStateChange is ever called. It seems runOnReadyStateChange is called at the end of the current tick, so IE never reaches the next tick to run the setTimeout callback. (After a few seconds the browser melts down, probably due to the ever-increasing log.)

@calvinmetcalf
Copy link

what happens with setTimeout(, 4) is the issue that IE isn't appropriately clamping setTimeout

@calvinmetcalf
Copy link

your updating the dom before you reset the queue for runOnReadyStateChange but not for runSetTimeout only setTimeout runs in chrome

@ghost ghost assigned briancavalier Sep 27, 2013
@briancavalier
Copy link
Member

This is in 6a7755e. We could release it in 2.4.1. Any concerns before we do?

@briancavalier
Copy link
Member

Merged to dev and master. Will go out in 2.4.1 soon.

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

5 participants