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

Infinite loop in Safari #79

Closed
mlenderink opened this issue Jun 15, 2017 · 11 comments
Closed

Infinite loop in Safari #79

mlenderink opened this issue Jun 15, 2017 · 11 comments

Comments

@mlenderink
Copy link

Got an infinite loop while using pow
Seems related to MikeMcl/bignumber.js#120

@MikeMcl
Copy link
Owner

MikeMcl commented Jun 16, 2017

Okay, thanks for the report. I'll take a look this weekend.

What version of iOS?
I think iOS 10.3.2 is the latest - is the bug still present there?

@mlenderink
Copy link
Author

I had this issue on Safari 10.1 on my mac using macOS 10.12.4

@MikeMcl
Copy link
Owner

MikeMcl commented Jun 28, 2017

Apologies for the delay.

Is the following true or false on the afflicted Safari?

Object.keys({ '250': 0.5, '1000': 0.1 }).length === 2

@mlenderink
Copy link
Author

false

@MikeMcl
Copy link
Owner

MikeMcl commented Jun 28, 2017

Thanks, Martin.

Please could you also tell me if running the following code fixes this issue

if (Object.keys({'250': 0.5, '1000': 0.1}).length !== 2) {
  Array.prototype.unshift = function () {
    Array.prototype.splice.apply(this, Array.prototype.concat.apply([0, 0], arguments));
    return this.length;
  };
  Array.prototype.shift = function () {
    return Array.prototype.splice.call(this, 0, 1)[0];
  };
}

@taylor-cedar
Copy link

@MikeMcl That code works, but I don't think it's a good idea to change the Array prototype in a library. Other libraries may be depending upon other behavior. A better method is something like this.

var shift, unshift;
if (Object.keys({'250': 0.5, '1000': 0.1}).length == 2) {
  unshift = Function.prototype.call.bind(Array.prototype.unshift)
  shift = Function.prototype.call.bind(Array.prototype.shift)
} else {
  unshift = function(arr, /*new array elements */) {
    Array.prototype.splice.apply(arr, Array.prototype.concat.apply([0, 0], Array.prototype.slice.call(arguments, 1));
    return arr.length
  }
  shift = function (arr) {
    return Array.prototype.splice.call(arr, 0, 1)[0];
  };
}

And then you can use the shift and unshift methods without affecting the prototype.

shift(myArr)

unshift(myArr, 3)

@MikeMcl
Copy link
Owner

MikeMcl commented Jul 6, 2017

@taylor-cedar

Thanks for testing the code and for offering another possible solution.

My preference would be to not have to make changes to my libraries to work around bugs elsewhere.

Changing the prototype seems reasonable to me here because if the bug is present then the built-in shift and unshift implementations stop arrays working reliably, not just in this library but everywhere.

For example, if the bug is present

var arr = [0, 2147483648];
arr.shift(); 
arr[1] = 1;
// arr should be [2147483648, 1] but is [2147483648] in Safari

@taylor-cedar
Copy link

taylor-cedar commented Jul 6, 2017

I understand, but think about if every library owner has the same idea. Most code bases use 50-100 libraries. If every library overrides the Array prototype, it will be very inefficient and very unpredictable.

@benfletcher
Copy link

FWIW, Safari 11.0 on Mac doesn't have this issue. It may also be fixed on 10.x updates. 11 is due for imminent release with new High Sierra update.

@MikeMcl
Copy link
Owner

MikeMcl commented Sep 23, 2017

@numbergames

Thanks for the info.

@Xotic750
Copy link

Xotic750 commented Oct 1, 2017

I was having a look to see if this could be globally fixed with JavaScript

es-shims/es5-shim#449

I didn't find a global fix, but I did come up with something that may be useful, though I can't test it as I don't have access to Safari.

https://github.com/Xotic750/create-array-fix-x

Link to tests

https://rawgit.com/Xotic750/create-array-fix-x/master/tests/index.html

@MikeMcl MikeMcl closed this as completed Sep 25, 2018
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