#16425 closed defect (fixed)
[patch][ccla] IE10: JS engine bug causes dojo/aspect. to fail
Reported by: | Eric Durocher | Owned by: | Kris Zyp |
---|---|---|---|
Priority: | blocker | Milestone: | 1.7.5 |
Component: | Events | Version: | 1.8.1 |
Keywords: | ie10 | Cc: | cjolif, Adrian Vasiliu |
Blocked By: | Blocking: |
Description
This seems to be an IE10 JS engine / JIT bug.
The context where this occurs is in Dojo Mobile transitions and can be reproduced (for example) by running this test case on IE10:
dojox/mobile/tests/test_RoundRectList.html
Click the "Slide" list item, the view becomes empty instead of transitioning to another view.
After debugging this, I spotted the problem in dojo/aspect.js, more precisely lines 53-59, where the JS engine seems to get confused with the 'previous' variable and not update it correctly in some cases, and then fail to add one of the after advices in the list (which in turn causes one of the dojox/mobile slide animations to not run - I can give the details of the call chain if needed).
This is not easy to debug directly since the bug does not occur when the IE10 debugger is active. The reasons why I am convinced that this is a JS engine bug:
- Changing the code slightly to something strictly equivalent fixes the problem. The attached patch is one variant that does not look too bad, but more stupid things like adding a "previous = previous;" statement in the loop at line 56 work too...
- Adding traces in a string (not through console.log calls since this also fixes the bug) shows that the value of 'previous' after the loop is not the same as the last value set in the loop.
- Does not happen in the debugger (where, I suppose, JIT is disabled)
I tried to reproduce in simpler cases, with no success. The bug can be reproduced in many Dojo Mobile tests/demos using the same code for transitions.
The proposed patch is just one possible fix, many other workarounds would work too.
The bug is absolutely not in dojo/aspect, the JIT just happens to fail here apparently, no idea why of course. Open to any ideas how to fix this differently...
Attachments (3)
Change History (26)
Changed 10 years ago by
Attachment: | ie10-aspect.patch added |
---|
comment:1 Changed 10 years ago by
Cc: | cjolif added |
---|
comment:2 Changed 10 years ago by
Keywords: | ie10 added |
---|---|
Milestone: | tbd → 1.8.2 |
Priority: | undecided → blocker |
Ideally this should be fixed in 1.8.2 to reasonably claim IE10 support for dojo mobile (and maybe other code that might fail against that). Please let us know what you think.
comment:3 Changed 10 years ago by
Summary: | IE10: JS engine bug causes dojo/aspect. to fail → [patch][ccla] IE10: JS engine bug causes dojo/aspect. to fail |
---|
comment:4 follow-up: 5 Changed 10 years ago by
i don't have IE10 handy... but i'm curious if this bug goes away if you take away the "use strict";
in this module.
comment:6 Changed 10 years ago by
I can't reproduce this, the Slide transition seems to work fine on the IE platform preview (version 1.9.8023.6000).
comment:8 Changed 10 years ago by
I implemented a slightly different, and more compact version of the linked list traversal that seems to avoid the bug. It does indeed appear to be some bizarre JIT bug in IE10. Let me know if this looks OK, and I can stick it in 1.8.
comment:9 Changed 10 years ago by
It fixes the failure for me (IE10, Windows 8).
Looking at the patch I wonder why we don't just keep a reference to the end of the linked listed, in addition to having a reference to the start, but I guess that's outside the scope of this ticket, especially for 1.8.
comment:12 Changed 10 years ago by
This issue has been reported upstream. I will update this ticket once I have more information.
comment:13 Changed 10 years ago by
Upstream is requesting a reduced test case for this issue. Could one of you please create one?
comment:14 Changed 10 years ago by
I spent some time working on this, but was never able to create anything isolated. However, if you go to the test page: http://download.dojotoolkit.org/release-1.8.1/dojo-release-1.8.1/dojox/mobile/tests/test_RoundRectList.html
You can reproduce the problem with a relatively small snippet of code on that page:
aspect = require('dojo/aspect'); var order = []; obj = { foo: function(){} }; aspect.after(obj, "foo", function(){order.push(1)}); aspect.after(obj, "foo", function(){order.push(2)}); aspect.after(obj, "foo", function(){order.push(3)}); obj.foo(); order
And the order array will only be [1,3], instead of [1,2,3] as it should.
I also tried running this snippet at different times in the loading of the page. When the page first starts the snippet runs correctly, but at some point, I think after the widgets have been rendered, the snippet starts running incorrectly.
And again, as mentioned, if you have developer tools script debugging turned on, the issue disappears. I had also tried removing components on the page to see if anything caused it. If you only have a couple widgets it goes away, but it is not any particular widget that causes the problem, it seems more related to just the number of widgets (yet more evidence of a low-level VM problem).
comment:15 Changed 10 years ago by
I was looking at backporting this to 1.7 but there's no test_RoundRectList.html in 1.7 to test the fix against. Is there some case in 1.7 that fails because of this bug, or some reproducible test case?
PS: it's not an issue in 1.6, because dojo/aspect didn't come until 1.7.
comment:16 Changed 9 years ago by
Cc: | Adrian Vasiliu added |
---|
Adrian might be able to provide sample reproducing the issue in Eric's absence.
comment:17 Changed 9 years ago by
The attached test_RoundRectList-dojo1.7.html is a slightly modified version of test_RoundRectList.html from trunk to make it runnable in Dojo 1.7. The test is to be placed in dojox/mobile/tests.
Using it, I do reproduce the bug in IE10 on Windows 8 using Eric's testing scenario (clicking the "Slide" list item, it transitions to an empty view).
Also tested that applying the patch to dojo/aspect the bug goes away.
comment:18 Changed 9 years ago by
Hmm, that's strange, it's working for me, see http://bill.dojotoolkit.org/1.7/dojox/mobile/tests/test_RoundRectList-dojo1.7.html.
comment:19 Changed 9 years ago by
For me, your URL does reproduce it. My test env.: IE 10.0.9200.16466 under a Win8 Virtual Machine. Holds for the first load after a clear cache, and for subsequent reloads as well.
Maybe running under a Virtual Machine "helps" reproducing...
comment:20 Changed 9 years ago by
I reproduce with the URL too, on Windows 7 (not a VM) / IE 10.0.9200.16438. The problem can also be seen in the mobile gallery demo (demos/mobileGallery), just clicking any item in the list.
comment:21 Changed 9 years ago by
I see, OK you guys should backport the fix then since you can reproduce the error.
Changed 9 years ago by
Attachment: | ie0-aspect-1.7.patch added |
---|
Backport for 1.7 including test case - Eric Durocher (IBM, CCLA), Adrian Vasiliu (IBM, CCLA)
comment:23 Changed 8 years ago by
Milestone: | 1.8.2 → 1.7.5 |
---|
Possible patch for IE10 / JIT problem - Eric Durocher (IBM, CCLA)