Summary: | [iOS8][ARMv7(s)] Optimized Object.create in 'use strict' context sometimes breaks. | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Stefan Penner <stefan.penner> | ||||
Component: | JavaScriptCore | Assignee: | Benjamin Poulain <benjamin> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Critical | CC: | benjamin, bugs.webkit.org, ddkilzer, dnelsen, fpizlo, ggaren, mark.hatsell, mjstaffa, msaboff, rshimazu, sivakumar.ur.friend, spencerwi, unwttng, webkit-bug-importer, ysuzuki | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | iPhone / iPad | ||||||
OS: | iOS 8.0 | ||||||
Attachments: |
|
Description
Stefan Penner
2014-10-23 21:13:06 PDT
So basically this doesn't affect all, but it affects a good amount of ember 1.7+ (since we added 'use strict') apps in the wild. Scouring stack overflow it appears several angular users have the same problem. I suspect any app that has a hot Object.create in a 'use strict' context could suffer from this. Typically the result is a hard failure, and a broken app experience. Also, for any ember users who stumble upon this. 1.7 -> 1.8beta5 (and those related canary builds) are affected as they include 'use strict' contexts There is a very strong probability that 1.8 final, will ship with 'use strict' stripped. But as a stop-gap, I am maintaining 1.7 and canary versions of ember that have 'use strict stripped' and do not appear to have this issue. 1.7: ---- bower users: -> yapplabs/ember-ios8-fix#1.7.0 https://github.com/yapplabs/ember-ios8-fix/tree/1.7.0 canary: ------- bower users: -> yapplabs/ember-ios8-fix https://github.com/yapplabs/ember-ios8-fix/tree/master Do we have a way to reproduce this failure (even if not a reduced way)? The JSBIn's included in DEMO should consistently fail on the affected devices. I can also make myself available for further discussion/questions if needed. This is also affecting angularjs applications. Is this being worked on or is there something we can do to speed up the process? +1 this is affecting our app. Response has been to remove use strict declarations but that's pretty undesirable. Is any work going on for the radar yet? Also for more information towards the original report, in our codebase instead of "setting properties results in a thrown exception" we were often seeing what I'd describe as "setting properties invisibly does not happen". At one point, code like: ```js this.data = {}; console.log(this.data); ``` Would output `undefined`. At other points it would crash Safari, and we would get the app reload with the grey notification bar. In almost 100% of cases it would cease any useful communication with Safari desktop dev tools. Lastly, great job pinning down a reproducible case for this Stefan, this is a seriously slippery bug in my recent experience. Still broken on ToT :( Update: we are still seeing the effects of this bug even after totally removing `'use strict';` declarations from our code, so as of the moment there's not a known workaround that is good for everyone. @jack remember that JSC will inline code. As far as I can tell, if a non-use strict module that contains Object.create is inlined into a use strict module the problem returns. With ember, we also still saw the issues until all potential Object.create inline sites in the framework where also stripped of `use strict`. This is extremely brittle, as we do not control user-land code. And if the code is slightly refactored we may once again regress. We cant just strip use strict from our entire program, as we rely on subtle differences. This really needs to get fixed, hopefully with the recent burst of attention it will happen sooner. Agreed, there's no even semi-good way round this. Good to see an @webkit edit to the ticket in the last few days, even if they didn't comment :( I likely haven't done a good job of bringing attention to this issue. So I appreciate the extra eyes and support its getting now. My appologies for not doing more earlier We (Amplify Education) are also seeing this bug on 32-bit iPads, manifesting as an angularjs error when creating a new object. Created attachment 253823 [details]
Patch
Comment on attachment 253823 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=253823&action=review r=me > Source/JavaScriptCore/ChangeLog:24 > + and we fail the put_by_id because we try to set a property a on number. Wording - "... a on number", should this be "on a number"? Committed r184960: <http://trac.webkit.org/changeset/184960> Great to see a resolution. Next question is when this will make its way into mobile Safari? Great to see a patch/resolution for this. I do echo Jack's question though: is there any word yet on a timeline for fixing iOS Safari? This causes sporadic/nondeterministic issues with Angular.js webapps, and there's no real workaround except to recommend installing Chrome for its JS engine. This was fixed in iOS 9 Dev Seed 2. (In reply to comment #21) > This was fixed in iOS 9 Dev Seed 2. Great! Thanks for the quick response. Are there any plans for a backport for iOS8 users? (pardon me if it's a question with an obvious answer; I'm not very familiar with the iOS ecosystem in general) (In reply to comment #22) > (In reply to comment #21) > > This was fixed in iOS 9 Dev Seed 2. > > Great! Thanks for the quick response. Are there any plans for a backport for > iOS8 users? (pardon me if it's a question with an obvious answer; I'm not > very familiar with the iOS ecosystem in general) It is unlikely that this will be backported to iOS 8.x. This jsfiddle example still seems to fail for me on iOS 9 Beta 2. http://jsfiddle.net/adrian_vasiliu/ftyafr0d/ > It is unlikely that this will be backported to iOS 8.x.
This would be unfortunate, a large amount of devices and sites are affected by this. The solution is basically, hand tailor scenario by scenario and wait for armv7 devices on iOS8 to age out.
Luckily iOS age out faster then android, Unfortunately this will still be years of time. Time where app authors have no reasonable solution.
(In reply to comment #24) > This jsfiddle example still seems to fail for me on iOS 9 Beta 2. > > http://jsfiddle.net/adrian_vasiliu/ftyafr0d/ This fiddle illustrates a different issue. Confusingly it manifests in a similar way as the one this bug is tracking. (In reply to comment #26) > (In reply to comment #24) > > This jsfiddle example still seems to fail for me on iOS 9 Beta 2. > > > > http://jsfiddle.net/adrian_vasiliu/ftyafr0d/ > > This fiddle illustrates a different issue. Confusingly it manifests in a > similar way as the one this bug is tracking. Do we have a new bug tracking this issue? (In reply to comment #25) > > It is unlikely that this will be backported to iOS 8.x. > > This would be unfortunate, a large amount of devices and sites are affected > by this. The solution is basically, hand tailor scenario by scenario and > wait for armv7 devices on iOS8 to age out. > > Luckily iOS age out faster then android, Unfortunately this will still be > years of time. Time where app authors have no reasonable solution. Although I'm happy that this is fixed, I feel concerned that this isn't getting backported to iOS8. For those of us serving enterprise web apps this is a blocking issue since most of our users just can't use our apps because of this issue. The 'use strict' removal didn't work for us. Not entirely sure about the webkit process but there have been instances where such issues have been fixed at least in one prior version. Given that iOS9 isn't still out and would take a while for everyone to adapt please consider backporting this. Thanks, Sivakumar. We will take this into consideration, but we rarely release updates to the previous major version of iOS after shipping a new major version. I would encourage you to continue looking for a workaround if possible. If I might, I'll register again that this is impacting a large swathe of our users too, with no attempts at workarounds working. If you're not prepared to backport, I feel a certain amount of support towards the effort of finding viable workarounds would be a responsible way to treat this. It's a breaking issue in a core part of code. |