Closed Bug 934889 Opened 11 years ago Closed 8 years ago

JS_InitStandardClasses fails intermittently on a web worker due to the Intl API

Categories

(Core :: JavaScript: Internationalization API, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: khuey, Assigned: khuey)

References

Details

Attachments

(1 file)

Web workers currently do not call JS_InitStandardClasses.  I added code to call it but the function fails intermittently.  When it does fail, it fails in IntlInitialize (http://hg.mozilla.org/mozilla-central/annotate/770de5942471/js/src/builtin/Intl.cpp#l396).

Filing this for a bug number to put in my comment.
Is it possible that intl init is somehow racy/unsafe off the main thread?
Intl init is basically plain old JS operations, plus some ICU calls.  ICU is supposed to be threadsafe such that the ICU calls will be okay.  (There are some things you have to do before using ICU at all, in u_init -- which we call at startup well before any sorts of racing things could happen.)

Catching this in a debugger is really what's desired here, at this point, because there's definitely too much under IntlInitialize to eyeball what's wrong.  :-\
Also because not everything is hooked up to JS_ResolveStandardClass, workers are missing some builtins.  e.g. bug 935165.
Blocks: es-intl
Anybody interested in looking into this?
Flags: needinfo?(mozillabugs)
Flags: needinfo?(jwalden+bmo)
Sorry, I'm working on other things now.
Flags: needinfo?(mozillabugs)
Mass-moving existing Intl-related bugs to the new Core :: JavaScript: Internationalization API component.

If you think this bug has been moved in error, feel free to move it back to Core :: JavaScript Engine.

[Mass change filter: core-js-intl-api-move]
Component: JavaScript Engine → JavaScript: Internationalization API
Kyle claims that workers resolve standard classes now, everywhere.  They didn't when this bug was filed, but since then with WebIDL progress that's changed, and now everyone does resolve them.  And if everyone's resolving them, that's enough for observably correct semantics.  Getting the properties via init, just means they show up earlier, but there's not really much reason to care about that if nobody's looking for them.

And indeed, this all comports with my testing demonstrating that Intl stuff works just fine on workers.  For example, this:

http://whereswalden.com/files/mozilla/intl-in-workers/intl-worker.html

So I am going to claim this is fine now.  There's still the code that http://mxr.mozilla.org/mozilla-central/search?string=bug%20934889 refers to, that should be cleaned up, to be sure.  But that's a much lesser concern than the one originally expressed here.
Flags: needinfo?(jwalden+bmo)
Attached patch PatchSplinter Review
This seems to work now (at least on my machine).
Assignee: nobody → khuey
Status: NEW → ASSIGNED
Attachment #8736376 - Flags: review?(bzbarsky)
Comment on attachment 8736376 [details] [diff] [review]
Patch

r=me

Do we still need resolve standard classes stuff?
Attachment #8736376 - Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/mozilla-central/rev/37bfc8717cf3
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.