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

Object formating is limitless => Problems with giantic arrays #492

Closed
Phaiax opened this issue Jun 10, 2014 · 4 comments · Fixed by sinonjs/formatio#6 or #525
Closed

Object formating is limitless => Problems with giantic arrays #492

Phaiax opened this issue Jun 10, 2014 · 4 comments · Fixed by sinonjs/formatio#6 or #525

Comments

@Phaiax
Copy link

Phaiax commented Jun 10, 2014

Hey

Using Sinon.JS asertions in browser mode with html formating and a failed assertion (eg calledTwice) causes Sinon to format (to string) all calls to the spied function. Thereby all objects will be formated recoursively. The formater prevents formations of already formated objects -> [circular reference], but it does not prevent the stringification of big arrays.

I'm using Phaser, a game framework. It has an internal array for the Canvas it's painting on. This Uint8ClampedArray has a default size of 1024 * 1024 * 4 = 4194304. I don't know if it's the only array, but the stringification of the array(s) takes more than 1GB of RAM and causes the script to "not respond".

You can't blame a lib to use large arrays, so Sinon.JS must handle this.

Simple example using Phaser to trigger the problem. It will also work if you just create a big array and try to format it:

var g = new Phaser.Game(400, 400, Phaser.AUTO, null, null); // only needed because BitmapData needs a Game Object
var o = {s : new Phaser.BitmapData(g, '__root', 1024, 1024)} ;

sinon.format(o);

I'll create a patch and attach it to this issue. I think arrays and object should have a maximum of 100..150 formated properties,

@Phaiax
Copy link
Author

Phaiax commented Jun 10, 2014

Ok, after a little bit of investigation i found, that sinon is using formatio for formating.

https://github.com/busterjs/formatio

Like sinon does with

    var formatter = formatio.configure({ quoteStrings: false });

you can configure formatio to behave special.
The cleanest solution would be to add a new config for the maximum array length to formatio and set it properly in sinon.

@Phaiax
Copy link
Author

Phaiax commented Jun 10, 2014

I created a patch for formatio, and I saw that you (@cjohansen) are also a maintainer of formatio, so it will be easy for you to merge my patch (if it's acceptable :).

Sinon.JS is missing the config, but it's just replacing

{ quoteStrings: false }
with
{ quoteStrings: false, limitChildrenCount: 250 }
in sinon.js line 366

Hope you'll fix this, thank you and kind regards
Phaiax

@mroderick
Copy link
Member

Hmm, this shouldn't be closed yet. We still need to use a new formatio version

@cjohansen cjohansen reopened this Jul 29, 2014
@cjohansen
Copy link
Contributor

Oops, sorry!

mroderick added a commit to mroderick/sinon that referenced this issue Jul 30, 2014
cjohansen added a commit that referenced this issue Aug 4, 2014
Fix #492 by upgrading formatio and setting a limit
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

Successfully merging a pull request may close this issue.

3 participants