Unknown issue with Chrome 54.0.2840.59 m (64-bit)

[MetoooRepo]

Hi Boris,

Yesterday my Google Chrome for Windows updated to version "54.0.2840.59 m" and i started to notice "strange things" on the project i'm working on with JsViews ( 0.9.81 ).

In particular some errors are thrown by the function convertVal(converter, view, tagCtx, onError) on the line 407:

value = tagCtx.args[0];

The error thrown is "Uncaught TypeError: Cannot read property '0' of undefined" and it happens because the variable tagCtx is sometimes an array and not a single value.

The same identical code works flawlessly on Google Chrome Canary 56.0.2890.0 and any other major browser i've tried ( Edge, Firefox, Safari )

Any clues?

I will try to reproduce the error on jsFiddle but it's a bit tricky because i've no idea of "what is happening" and what causes that error.

Regards, Salvatore

[Paul-Martin]

Hi Salvatore. This appears to be a regression in Chrome itself. See the the following issue:

https://bugs.chromium.org/p/chromium/issues/detail?id=656037&can=2&start=0&num=100&q=&colspec=ID%20Pri%20M%20Stars%20ReleaseBlock%20Component%20Status%20Owner%20Summary%20OS%20Modified&groupby=&sort=

The bug is caused by chrome intermittently returning the incorrect value from Array.push()

I have found a simple workaround is to modify jsviews.js (assuming latest version) to change buildCode line 1978 as follows:

if (tmplBindings && (pathBindings = node[7])) { // Array of paths, or false if not data-bound
     pathBindings = [pathBindings];
     tmplBindingKey = tmplBindings.push(1); // Add placeholder in tmplBindings for compiled function
     tmplBindingKey = tmplBindings.length;
}

This is a very bad chrome bug and I hope they will fix it quickly.

If you have other libraries or code that relies on the return value of Array.push() you may want to find those locations and make a similar change.

[BorisMoore]

OK - I have just installed that Chrome update. So far I have not hit the issue though. The jsviews.com site seems to run correctly on Chrome as far as I can see - though I have not navigated to all the site pages and samples...

So it looks as if the Chrome bug issue arises in certain JsViews apps, but not all, and that for each of you, your SPAs do hit the issue. Salvatore, if you insert the extra line that Paul suggests: tmplBindingKey = tmplBindings.length; does that stop the convertVal() error from getting thrown? It would be good to confirm whether your convertVal error scenario is indeed a consequence of the tmplBindingKey = tmplBindings.push(1) line in JsViews.

I have to decide whether to push a JsViews update with the defensive line of code included, but first I want to be sure that Salvatore's issue does come from that line of code.

[Paul-Martin]

Just to a add a bit more information. The issue occurs because the Chrome bug is to return whatever is being passed instead of returning the new length. For whatever reason this seems to only occur if the application is large enough. In the jsviews case this means it returns 1. Consequently any compiled functions stored in tmpl.bnds array get overwritten. Then when first render is attempted, you get a crash because of either a missing or incorrect compiled function in the array.

Also, as I'm sure you have now, I had run the full jsviews test suite on most recent release of chrome and was unable to trigger the chrome bug.

[BorisMoore]

Right, if I artificially replace the line tmplBindingKey = tmplBindings.push(1); with:

tmplBindings.push(1);
tmplBindingKey = 1;

then the test suite fails with 64 errors, but without that change, all pass. So in the context of the test suite, whenever tmplBindings.length > 0, tmplBindings.push(1) is indeed returning the correct value. (If the length is 0 to start with then of course the issue will not show up anyway, since the length and the pushed item are the same.)

So the error scenario in Chrome seems to only arise in the larger context of your SPAs. It will be interesting to get confirmation from Salvatore that his issue is indeed arising from the same tmplBinding.push(1) code (and the consequent overwriting of tmpl.bnds) ...

[Paul-Martin]

If you would like to see the error be reproduced, use this link. (Note that this is with an older version of JSViews - though this particularly logic is the same. I haven't had a chance to upgrade form some of the breaking changes.)

https://chromebug.workspace.com/cr656037/index.html?#/defect/table

This is modified to have

if (tmplBindingKey != tmplBindings.length)
   throw 'tmplBindings.push returned incorrect value = ' + tmplBindingKey + ' should be ' + tmplBindings.length;

Immediately after

tmplBindingKey = tmplBindings.push(1);

[BorisMoore]

Paul, thanks... I have been trying to debug the issue in your test page. I intermittently hit the issue. (If not I get a login dialog). Most times it has the following callstack:

where it is compiling the nested template that starts with {^{for approved() }}. But the final BuildCode execution is really weird:

• notice the undefined values such as:

It seems that tmplBinding.push(1) returned undefined, not 1, but it is difficult to see how tmplOptions can be undefined. It's as if the Chrome debugger state is completely broken.

I am hitting the error very intermittently when I refresh your test page, sometimes not at all for a while. And it seems never to throw if I have a debugger sessions going with active breakpoints. So very difficult to debug, and maybe related to some kind of underlying race condition in Chrome.

[Paul-Martin]

Yes. I should have mentioned that. The issue will never hit if the Debugger is active with breakpoints set. If I just have it open, but with no breakpoints set it will hit. Thats why I had the throw statement so that it stops. For me it seems to trigger more frequently - almost every time, but like you said this is likely some sort of race condition and thats likely due to the differences of our respective machines.

Its also worth pointing out that the sample code is code that had been running in production for months without change or issue. Only when Chrome updated did this issue occur. I spent a lot of time tracking that down. It was not easy to locate. And then another developer reported almost the same issue in the Promises library on the Chrome issue tracker.

[BorisMoore]

Maybe it would be worth calling out these aspects on the Chrome issue in their tracker...

Also maybe let them know that when they hit your test page and get the login dialog, this means the bug did not get encountered that time - and that the issue does indeed occur before that dialog comes up. (So they don't give up ... thinking that they need credentials to run the test...)

[MetoooRepo]

Salvatore, if you insert the extra line that Paul suggests: tmplBindingKey = tmplBindings.length; does that stop the convertVal() error from getting thrown?

Yes. Now i no longer see any error or strangeness. I'll keep on testing though, as my app is quite large at the moment.

Thank you all, guys!

[Paul-Martin]

If everybody affected by this issue could up vote it to be fixed on the chrome issue tracker I think that would be very helpful. Also if you have any comments on the behavior you are seeing in your environment that might be helpful to add to the chrome issue tracker as well. Here is a direct link:

https://bugs.chromium.org/p/chromium/issues/detail?id=656037&can=2&start=0&num=100&q=&colspec=ID%20Pri%20M%20Stars%20ReleaseBlock%20Component%20Status%20Owner%20Summary%20OS%20Modified&groupby=&sort=

[Paul-Martin]

FYI - this defect has been fixed in Chrome version 54.0.2840.71 which was released today. If you wish to validate that you have the fixed version of chrome there is a snippet of javascript code that can be run in the dev tools console. See comment #24. https://bugs.chromium.org/p/chromium/issues/detail?id=656037&can=2&start=0&num=100&q=&colspec=ID%20Pri%20M%20Stars%20ReleaseBlock%20Component%20Status%20Owner%20Summary%20OS%20Modified&groupby=&sort=

[BorisMoore]

@Paul-Martin - thanks for following through on this, and getting rapid resolution.