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 makes the process crash #3111

Closed
targos opened this issue Sep 29, 2015 · 15 comments
Closed

Calling Intl.v8BreakIterator makes the process crash #3111

targos opened this issue Sep 29, 2015 · 15 comments
Assignees
Labels
i18n-api Issues and PRs related to the i18n implementation. v8 engine Issues and PRs related to the V8 dependency.

Comments

@targos
Copy link
Member

targos commented Sep 29, 2015

node -p "new Intl.v8BreakIterator()"


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

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

 1: V8_Fatal
 2: v8::internal::BreakIterator::InitializeBreakIterator(v8::internal::Isolate*, v8::internal::Handle<v8::internal::String>, v8::internal::Handle<v8::internal::JSObject>, v8::internal::Handle<v8::internal::JSObject>)
 3: v8::internal::Runtime_CreateBreakIterator(int, v8::internal::Object**, v8::internal::Isolate*)
 4: 0x35e1cb4060bb
[1]    14295 illegal hardware instruction (core dumped)  node -p "new Intl.v8BreakIterator()"

This is the line where it happens:

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

I guess that the feature v8 is trying to load is not available with small-icu ? Anyway it seems wrong to experience a fatal error in this case.

@targos targos added v8 engine Issues and PRs related to the V8 dependency. i18n-api Issues and PRs related to the i18n implementation. labels Sep 29, 2015
@evanlucas
Copy link
Contributor

Yea, I'm seeing this also. It works when built with full-icu, but fails with small-icu

@bnoordhuis
Copy link
Member

/cc @nodejs/intl

@srl295
Copy link
Member

srl295 commented Sep 30, 2015

V8 wanted this to be a fatal instead of just a soft error (exception or
something. )

Yes small Icu doesn't have the data needed for this undocumented feature.
Takes significant space- Use full Icu.

If this is an important feature lets get it added to ecma402

El martes, 29 de septiembre de 2015, Ben Noordhuis notifications@github.com
escribió:

/cc @nodejs/intl https://github.com/orgs/nodejs/teams/intl


Reply to this email directly or view it on GitHub
#3111 (comment).

@srl295
Copy link
Member

srl295 commented Oct 6, 2015

Should we "delete" the Intl.v8BreakIterator property if we know at startup that it can't function? Thoughts anyone?

@caridy
Copy link

caridy commented Oct 6, 2015

Does anyone know what Intl.v8BreakIterator does?

Maybe @ajklein knows more about this method.

@ajklein
Copy link
Contributor

ajklein commented Oct 6, 2015

@nciric would know much better than I (this is the best I could find: https://code.google.com/p/v8-i18n/wiki/BreakIterator).

@nciric
Copy link

nciric commented Oct 7, 2015

Break iterator does character/sentence segmentation in a locale friendly way. It relies on largish data files, and that's one of the reasons we added it to the Intl.* for Chrome/V8. Intl.v8BreakIterator is not part of ECMA 402 spec.

Feel free to remove support for it from node. V8 may needed it for some internal projects.

@Fishrock123
Copy link
Contributor

Should we "delete" the Intl.v8BreakIterator property if we know at startup that it can't function? Thoughts anyone?

Nah just make it throw.

@srl295 srl295 self-assigned this Oct 24, 2015
@srl295
Copy link
Member

srl295 commented Oct 24, 2015

OK. Current thought:

  • if the user sets a data path using env var or parameter (or in detect "full-icu" module #3460 full ICU is detected) , set process.icu_data_dir to that data path in node.cc
  • in node.js:
       if (process.config.variables.icu_small && !process.icu_data_dir) {
          Intl.v8BreakIterator=function(){throw Error('Data not available…');}
      }

@caridy
Copy link

caridy commented Oct 24, 2015

Does node exposes any other v8 specific functionality? I'm worry about portability of the code if you use v8BreakIterator().

@srl295
Copy link
Member

srl295 commented Oct 27, 2015

@caridy no, nothing like that is exposed.

@caridy
Copy link

caridy commented Oct 27, 2015

I will say then, make it throw, and lets work on getting the break iterator in 402.

@jasnell
Copy link
Member

jasnell commented Oct 27, 2015

+1 to making it throw until it's part of 402

@srl295
Copy link
Member

srl295 commented Oct 28, 2015 via email

srl295 added a commit to srl295/node that referenced this issue May 3, 2016
If the undocumented v8BreakIterator does not have data available,
monkeypatch it to throw an error instead of crashing.

Fixes nodejs#3111
@srl295 srl295 closed this as completed in cd752e8 May 4, 2016
@srl295
Copy link
Member

srl295 commented May 4, 2016

$ node -p "new Intl.v8BreakIterator()"
{}

evanlucas pushed a commit that referenced this issue May 17, 2016
If the undocumented v8BreakIterator does not have data available,
monkeypatch it to throw an error instead of crashing.

Fixes: #3111
PR-URL: #4253
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
srl295 added a commit to srl295/node that referenced this issue Oct 10, 2016
If the undocumented v8BreakIterator does not have data available,
monkeypatch it to throw an error instead of crashing.

Backport of nodejs#3111 c99f231
MylesBorins pushed a commit that referenced this issue Oct 10, 2016
If the undocumented v8BreakIterator does not have data available,
monkeypatch it to throw an error instead of crashing.

Fixes: #3111
Ref: #9008
PR-URL: #4253
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
MylesBorins pushed a commit that referenced this issue Oct 10, 2016
If the undocumented v8BreakIterator does not have data available,
monkeypatch it to throw an error instead of crashing.

Fixes: #3111
Ref: #9008
PR-URL: #4253
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
rvagg pushed a commit that referenced this issue Oct 18, 2016
If the undocumented v8BreakIterator does not have data available,
monkeypatch it to throw an error instead of crashing.

Fixes: #3111
Ref: #9008
PR-URL: #4253
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
MylesBorins pushed a commit that referenced this issue Oct 26, 2016
If the undocumented v8BreakIterator does not have data available,
monkeypatch it to throw an error instead of crashing.

Fixes: #3111
Ref: #9008
PR-URL: #4253
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
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.
Projects
None yet
Development

No branches or pull requests

9 participants