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

Calling Intl.v8BreakIterator w/o ICU data in VM context makes the process crash #14909

Closed
TimothyGu opened this issue Aug 18, 2017 · 4 comments
Closed
Labels
i18n-api Issues and PRs related to the i18n implementation. v8 engine Issues and PRs related to the V8 dependency. vm Issues and PRs related to the vm subsystem.

Comments

@TimothyGu
Copy link
Member

TimothyGu commented Aug 18, 2017

#3111 / #4253 fixed this in the main context, but the bug applies to VM contexts too unfortunately.

$ node -p "vm.runInNewContext('new Intl.v8BreakIterator()')"


#
# Fatal error in , line 0
# Failed to create ICU break iterator, are ICU data files missing?
#

==== C stack trace ===============================

    node(v8::base::debug::StackTrace::StackTrace()+0x16) [0x15c8c36]
    node() [0x1451857]
    node(V8_Fatal+0xd8) [0x15c3998]
    node() [0x1116487]
    node(v8::internal::Runtime_CreateBreakIterator(int, v8::internal::Object**, v8::internal::Isolate*)+0x170) [0x1200a40]
    [0x1ec0a1974998]
Illegal instruction

Offending line:

FATAL("Failed to create ICU break iterator, are ICU data files missing?");

At this point I'm thinking of just including the data files needed for the break iterator to work even if it's not part of the ECMA-402 spec yet.

/cc @nodejs/intl @srl295

@mscdex mscdex added i18n-api Issues and PRs related to the i18n implementation. vm Issues and PRs related to the vm subsystem. v8 engine Issues and PRs related to the V8 dependency. labels Aug 18, 2017
@bnoordhuis
Copy link
Member

That won't help in non-intl builds, though. In retrospect, we should have sent a patch upstream to turn that abort into an exception, instead of the hack from #4253. Shouldn't be too hard, I'll try that later today.

@TimothyGu
Copy link
Member Author

Non-Intl builds don't include this function so there's no issue there.

@bnoordhuis
Copy link
Member

Sorry, I mean --with-intl=system builds.

@bnoordhuis
Copy link
Member

bnoordhuis added a commit to bnoordhuis/io.js that referenced this issue Sep 8, 2017
It was never an official Ecma-402 API, it is about to be superseded
by `Intl.Segmenter` and it's prone to crash under some circumstances.

Searches don't turn up any usage in the wild and the recommendation
from the V8 team is to remove it.  Now seems like a good a time as
any to do that.

Fixes: nodejs#8865
Fixes: nodejs#14909
Refs: https://github.com/tc39/proposal-intl-segmenter
Refs: https://chromium-review.googlesource.com/c/v8/v8/+/620755
addaleax pushed a commit to addaleax/node that referenced this issue Sep 13, 2017
It was never an official Ecma-402 API, it is about to be superseded
by `Intl.Segmenter` and it's prone to crash under some circumstances.

Searches don't turn up any usage in the wild and the recommendation
from the V8 team is to remove it.  Now seems like a good a time as
any to do that.

Fixes: nodejs#8865
Fixes: nodejs#14909
Refs: https://github.com/tc39/proposal-intl-segmenter
Refs: https://chromium-review.googlesource.com/c/v8/v8/+/620755
PR-URL: nodejs#15238
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
i18n-api Issues and PRs related to the i18n implementation. v8 engine Issues and PRs related to the V8 dependency. vm Issues and PRs related to the vm subsystem.
Projects
None yet
Development

No branches or pull requests

3 participants