
iOS 12 Safari Array reverse bug - wonderfuly
https://stackoverflow.com/questions/52390368/array-state-will-be-cached-in-ios-12-safari-is-bug-or-feature/52392901
======
pjc50
Finally we've discovered why people are asked in interviews to reverse an
array.

------
ArmandGrillet
I can also see the bug on macOS with Safari 12.0.

I find it quite worrying that devs already wrote a lib to fix the issue but
didn't fill a bug report to Apple and shared it on SO. Anyway, a trendy HN
post might be enough to get the attention of some Apple devs and I've pinged a
dev just in case:
[https://twitter.com/ArmandGrillet/status/1042339847384518656](https://twitter.com/ArmandGrillet/status/1042339847384518656)

~~~
BonesJustice
Does Apple have a halfway decent public issue tracker? I don’t recall ever
seeing one, but I’ve never gone out of my way to look for one either.

~~~
smackfu
I would say it’s exactly “halfway decent”. There’s very little feedback and
bugs tend to get closed as duplicates but since it’s not public you can’t
monitor the state of the duolicates.

~~~
zwerdlds
Having had direct experience with this, this is the reason people don't file
bugs with Apple. It is an exercise in futility.

~~~
RKearney
I reported a UX bug with the new iOS feature Screen Time when the first
developer beta hit. Through the entire beta releases the only updates I got
were “we pushed a new release, can you test again?”.

Needless to say the bug made it to the final release of iOS 12. I’m done
filling out bug reports for this reason.

~~~
iforgotpassword
Same here. I have a little "blog" where I collect such things (with workaround
if any), and then hope google would redirect people there suffering the same
issue.

------
peterkelly
And now we'll have array-reverse-polyfill showing up as a 6th-level npm
dependency on our projects for years to come.

~~~
Aardwolf
Really? Why not write a for loop instead?

~~~
todd3834
Much easier to add a polyfill that uses a for loop than to refactor a large
codebase.

------
smaili
It appears this was already reported and fixed in WebKit -
[https://bugs.webkit.org/show_bug.cgi?id=188794](https://bugs.webkit.org/show_bug.cgi?id=188794)
but not specifically for the Safari browser.

~~~
stuartd

      Array.prototype.reverse modifies JSImmutableButterfly
    

You have to wonder how an immutable butterfly can be modified?!

~~~
taspeotis
I would be most grateful if someone could please explain to me what
"butterfly" is meant to be ... a metaphor of some sort I assume.

~~~
detaro
Internal data structure of JavaScriptCore representing objects, presumably
called a butterfly because it starts in the middle and spreads in both
directions in memory. One side is properties of the object, the other members
of its array aspect.

See e.g.
[http://phrack.org/papers/attacking_javascript_engines.html](http://phrack.org/papers/attacking_javascript_engines.html)
section 1.2

------
dan-robertson
This bug is very much like the most common mistake I see on Common Lisp stack
overflow where one tries to mutate constant (quoted) data.

However JavaScript has no similar notion of constant data but implementations
try to infer it as an optimisation. In this case that inference was wrong.

I think the memory structure looks something like:

    
    
      arr -> box1 -> 1,2,3,4
    

Reversing:

    
    
      arr -> box1 -> 4,3,2,1
    

But the data occupying the same region of memory before and after the sort. So
then when the page is refreshed the JavaScript doesn’t change and the literal
data must be included in the “parsed/compiled” form that is reused and so it
is initialised as

    
    
      arr -> box2 -> 4,3,2,1
    

The correct behaviour should be as follows:

    
    
      1. arr -> box1 -> loc1: 1,2,3,4
      2. Reverse
      3. arr -> box1 -> loc2: 4,3,2,1
      4. Refresh
      5. arr -> box2 -> loc1: 1,2,3,4
    

The “box” corresponds to the JavaScript object for the array (so mutating the
array data can change the box, the data it points at, but not just the
reference “arr” as the object might be pointed to from elsewhere). And this
box has another pointer to the data for the array (and presumably some bit
flag to say whether that is copy-on-write or not). This allows for more
efficient array functions when the data doesn’t actually change

------
0x0
The weirdest thing about this is the fact that the bug occurs after a page
reload. Does safari cache jit code and the javascript heap/objects between
pageloads?

~~~
olliej
When JavaScript is parsed by jsc, it’s compiled to a cachable bytecode, this
includes gc allocated constants for constant expressions - numbers, strings,
and the _underlying_ storage for some complex types like arrays and regexps.

The cachable code is kept in a big function source->bytecode hash table.

The cacheable bytecode is then “linked” to a bytecode that is optimized for
execution speed. That bytecode has things like create_array, etc that can take
the _immutable_ backing store object we generated earlier.

This means that if you have multiple functions with identical source code you
only have to do the expensive parse+compile step once, and you end up with
multiple independent functions using the same constant/immutable backing
stores. This saves memory and helps performance.

Unfortunately it adds complexity - now the mutable is objects have “immutable”
backing stores, so you have to implement copy-on-write semantics in order to
not share state between different instances of the linked code. In this case
it appears that a required CoW check was missing :(

------
kbumsik
I think Apple has to consider separating Safari (and other bundled apps) from
the iOS release so that users can update a quick fix through the App Store
without waiting for a new iOS update.

There was a serious WebAssembly regression in iOS 11.2 that makes wasm
effectively useless [1]. Devs had to disable wasm and wait for _months_ until
iOS 11.3 is released.

Apple never releases a iOS hot fix just for a single Safari bug. Then why
Apple don't let users to update Safari separately?

[1]:
[https://bugs.webkit.org/show_bug.cgi?id=181781](https://bugs.webkit.org/show_bug.cgi?id=181781)

~~~
favorited
Because this isn't "a single Safari bug." It's a bug in JavaScriptCore, which
is a public system framework. Safari uses it, but so does any other app which
links against it.

Fixing this bug requires shipping new versions of system frameworks, which is
pretty much the definition of an OS update.

Or, they could bundle a separate version of the WebKit frameworks just for
Safari (like the Safari Technical Preview does) – but then loading a website
in your app would use a different stack of frameworks than loading that site
in Safari, and no one wants that either.

~~~
kbumsik
That makes sense. I didn't know the Apple's system framework structure. Thanks
for the explanation.

------
0x0
Bug does not trigger on "Safari Technology Preview Release 65 (Safari 12.1,
WebKit 13607.1.5.2)" on macOS 10.13.6. Fixed already, or not yet introduced?

~~~
pvg
The shipping Safari is 606, the preview Safari is 607. So 'not yet introduced'
seems unlikely.

~~~
ash_gti
It appears to be fixed at HEAD and in Safari Technology Preview per
[https://bugs.webkit.org/show_bug.cgi?id=188794](https://bugs.webkit.org/show_bug.cgi?id=188794)

------
kuon
This sounds like a very bad bug, I wonder how bad it is "in the wild".

~~~
dan-robertson
I suspect bad. Literal data is common. Reversing is (reasonably) common. The
spec required that whenever one constructs data with a literal the resulting
object will have a different identity, and that (regular) arrays with
different identities do not alias.

------
fuzzy2
I wonder if `splice`, `shift`, `unshift` and other methods that modify arrays
in-place are also affected.

~~~
dartf
I've played with it a bit and no it doesn't. Also any modification to the
original array before calling `reverse` makes bug disappear

~~~
fuzzy2
Good to know, thanks for testing!

------
dkmar
Woah. I think I ran into this last night after updating to Safari 12 on macOS.
I was typing my message into messenger.com (facebook messenger), and all of my
typing was reversed. I even took a photo of it

~~~
Razengan
Are you sure it wasn't accidental Right-to-Left input?

------
chearon
I'm not seeing Safari 12 in the App Store (macOS 10.13.6). I wonder if they
pulled the update because of this? I have Safari 11 still but my colleague got
Safari 12 through the App Store.

~~~
olliej
Safari updates on Mac come down as part of system updates, so until the Mojave
timeframe updates appear

~~~
randomsofr
I got the update for Safari yesterday, and i'm still running High Sierra

~~~
olliej
Safari and webkit updates are pushed out to at least one, maybe two? (not sure
exactly how many) major releases.

------
toxik
Isn't this part of the faster loading that some Web browsers do? Essentially
the idea is that the browser muddies the idea of a closed page in favour of
being able to restore it faster.

~~~
gsnedders
It shouldn't be black-box observable like this.

------
cryptonector
Should we hold off on updating then?

------
matkinz
>I wrote a lib to fix the bug. [https://www.npmjs.com/package/array-reverse-
polyfill](https://www.npmjs.com/package/array-reverse-polyfill)

everyday we stray further from god

~~~
vanderZwan
What is extra frustrating is that this is an answer _StackOverflow_. The whole
point is to give clear, educational explanations. Well, the solution is only
21 lines, so why not explain the code snippet instead?

    
    
        (function() {
          function buggy() {
            function detect() {
              var a = [0, 1];
              a.reverse();
              return a[0] === 0;
            }
            return detect() || detect();
          }
          if(!buggy()) return;
          Array.prototype._reverse = Array.prototype.reverse;
          Array.prototype.reverse = function reverse() {
            if (Array.isArray(this)) this.length = this.length;
            return Array.prototype._reverse.call(this);
          }
          var nonenum = {enumerable: false};
          Object.defineProperties(Array.prototype, {
            _reverse: nonenum,
            reverse: nonenum,
          });
        })();
    

Look at how elaborate the repo is by comparison:

[https://github.com/fanmingfei/array-reverse-
ios12](https://github.com/fanmingfei/array-reverse-ios12)

~~~
KyleDavidE
? The snippet you showed is literally the exact code of the module:
[https://github.com/fanmingfei/array-reverse-
ios12/blob/maste...](https://github.com/fanmingfei/array-reverse-
ios12/blob/master/index.js)

The rest is just tests / npm config / license

~~~
vanderZwan
When I posted this, the snippet was not part of the answer being discussed.
There was only a link to a node module.

------
sitepodmatt
Fixed. Scheduled. Coming to iOS 15 in 2022...

~~~
saagarjha
This has already been fixed in Safari Technology Preview, and Safari usually
tails a release behind.

~~~
hajile
Yay for the incredibly slow bi-yearly update schedule.

~~~
saagarjha
Safari updates significantly more often than twice a year.

------
symlinkk
Does Apple not believe in unit tests or something? This and the password login
bug from a few months ago make me pretty concerned.

~~~
olliej
Webkit has hundreds of thousands of tests running on every check in (check
build.webkit.org iirc).

Checking arbitrary complex interactions isn’t trivial so more complex classes
of bugs can only get testing when they’re discovered.

------
vishalsharma
Now they will release iOS 13 for this :)

P.S.
[https://www.youtube.com/watch?v=uG8bNDw6Ftc](https://www.youtube.com/watch?v=uG8bNDw6Ftc)

------
maxwellito
Ironically, I got an ad banner for Apple at the top of this page. Saying
"Engineer in Europe. Innovate at Apple."

However, this bug sounds pretty serious! :-S

