Closed Bug 994459 Opened 10 years ago Closed 10 years ago

B2G l10n nightly bustage from "too much recursion" error (make[6]: *** [webapp-optimize] Error 3 and other errors)

Categories

(Firefox OS Graveyard :: Gaia::L10n, defect)

defect
Not set
blocker

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: RyanVM, Assigned: zbraniecki)

References

Details

(Keywords: intermittent-failure, regression)

Attachments

(2 files)

This either needs immediate fixing or bug 914414 needs to be backed out.

https://tbpl.mozilla.org/php/getParsedLog.php?id=37533183&tree=Mozilla-Central

b2g_mozilla-central_linux32_gecko_localizer nightly on 2014-04-09 16:09:26 PDT for push 690c810c8e3e
slave: bld-linux64-spot-120

Exception: InternalError: too much recursion

make[6]: *** [webapp-optimize] Error 3
make[5]: *** [libs] Error 2
make[6]: Leaving directory `/builds/slave/m-cen-linux32_g_lz-ntly-000000/build/gaia'
make[5]: Leaving directory `/builds/slave/m-cen-linux32_g_lz-ntly-000000/build/obj-firefox/b2g/gaia'
make[4]: *** [b2g/gaia/libs] Error 2
make[4]: Leaving directory `/builds/slave/m-cen-linux32_g_lz-ntly-000000/build/obj-firefox'
make[3]: *** [libs] Error 2
make[3]: Leaving directory `/builds/slave/m-cen-linux32_g_lz-ntly-000000/build/obj-firefox'
make[2]: *** [default] Error 2
make[2]: Leaving directory `/builds/slave/m-cen-linux32_g_lz-ntly-000000/build/obj-firefox'
make[1]: *** [realbuild] Error 2
make[1]: Leaving directory `/builds/slave/m-cen-linux32_g_lz-ntly-000000/build'
make: *** [build] Error 2
Flags: needinfo?(stas)
It looks like all of the locale-processing code is synchronously generating notifications and the base logic in webapp-optimize.js is just looping through all of the defined locales it knows about:
https://github.com/mozilla-b2g/gaia/blob/master/build/webapp-optimize.js#L573

Per my grep, it has taken care of 41 locales before in 423 frames before it blows the 512k stack limit it has on (non-debug) 32-bit linux.

A setTimeout/do_execute_soon would probably go a long way in that code.  (Specifically, don't touch the setter on https://github.com/mozilla-b2g/gaia/blob/master/build/webapp-optimize.js#L585 in the same loop cycle that dispatchEvent is being called.)
looking into it
can't reproduce it on my local machine with 62 locales on macos. Is it possible that we have different stack limit per platform?

The logic around locale cycling didn't change, but the stack have got deeper.
Yes, you have a 7 megabyte stack on OS X.  You will be unable to reproduce.
http://dxr.mozilla.org/mozilla-central/source/js/xpconnect/src/XPCJSRuntime.cpp#3051
One way to get it out of a single loop would be to just launch each app separately in https://github.com/mozilla-b2g/gaia/blob/master/build/webapp-optimize.js#L706

But there's no setTimeout in that code. Any idea what else can I use?
Attached patch patchSplinter Review
This code does exactly the same logic, but skips the pseudo-event dispatching.

It capitalizes on the fact that resource loading in buildtime mode is synchronous.
Assignee: stas → gandalf
Status: NEW → ASSIGNED
Attachment #8404483 - Flags: review?(stas)
We should overall clean webapp-optimize use of l10n.js. I filed bug 994520.
I can confirm based on inspection that this should address the problem as I understood it from the backtraces.

Specifically, the problem used to be caused by a write to:
  mozL10n.language.code = l10nLocales[processedLocales];
which would then do:
  navigator.mozL10n.ctx.requestLocales(lang);

The revised patch directly calls:
  lang = l10nLocales[processedLocales];
  mozL10n.ctx.requestLocales(lang);

And the control flow has been refactored so that the same effective linear code-path is the same, but without the recursion.

So r=asuth for what it's worth assuming travis is green.
I'm taking over while Gandalf is asleep on the west coast.  I'll create a pull request soon.  Thanks everyone for investigating this.
Flags: needinfo?(stas)
Comment on attachment 8404483 [details] [diff] [review]
patch

Review of attachment 8404483 [details] [diff] [review]:
-----------------------------------------------------------------

r=me.  I'll create a pull request for you and ask Andrew and Vivien for a review.

::: build/webapp-optimize.js
@@ +570,4 @@
>      };
>    };
>  
> +  win.dispatchEvent = function() {};

I'd move that to the beginning of the file where win is defined.

@@ +596,5 @@
> +          processedLocales + '/' + l10nLocales.length);
> +
> +        lang = l10nLocales[processedLocales];
> +
> +        mozL10n.ctx.requestLocales(lang);

I think this can simply read:

mozL10n.ctx.requestLocales(l10nLocales[processedLocales]);
Attachment #8404483 - Flags: review?(stas) → review+
Attached file Pull request
Attachment #8404638 - Flags: superreview?(21)
Attachment #8404638 - Flags: review?(bugmail)
Comment on attachment 8404638 [details] [review]
Pull request

Noting that I don't really have review authority for this code, (although for a nightly bustage fix I think the bar is a lot lower), this looks substantially the same as the other attachment except for the movement of dispatchEvent, so I'll give it my prestigious r=asuth too.
Attachment #8404638 - Flags: review?(bugmail) → review+
Thanks, Andrew :)

The Travis build is green: https://travis-ci.org/mozilla-b2g/gaia/builds/22685615
Comment on attachment 8404638 [details] [review]
Pull request

Alexandre, can you take a look?  Thanks!
Attachment #8404638 - Flags: superreview?(21) → review?(poirot.alex)
Comment on attachment 8404638 [details] [review]
Pull request

Looks good, just see my one comment in the pull request.
Attachment #8404638 - Flags: review?(poirot.alex) → review+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: