parcel-bundler/source-map

Crash: napi_create_reference

mischnic opened this issue · 4 comments

https://github.com/parcel-bundler/parcel/pull/4313/checks?check_run_id=543074718#step:3:366

FATAL ERROR: Error::Error napi_create_reference
 1: 0x9bcb80 node::Abort() [/usr/local/bin/node]
 2: 0x9bdd16 node::OnFatalError(char const*, char const*) [/usr/local/bin/node]
 3: 0x9bde09  [/usr/local/bin/node]
 4: 0x99600b napi_fatal_error [/usr/local/bin/node]
 5: 0x7f4f742c75b6  [/usr/src/app/.tmp/parcel-v2/node_modules/@parcel/source-map/build/Release/sourcemap.node]
 6: 0x7f4f742c780f Napi::Error::New(napi_env__*) [/usr/src/app/.tmp/parcel-v2/node_modules/@parcel/source-map/build/Release/sourcemap.node]
 7: 0x7f4f742c8627 Napi::Object::Get(char const*) const [/usr/src/app/.tmp/parcel-v2/node_modules/@parcel/source-map/build/Release/sourcemap.node]
 8: 0x7f4f742bee02 SourceMapBinding::addIndexedMappings(Napi::CallbackInfo const&) [/usr/src/app/.tmp/parcel-v2/node_modules/@parcel/source-map/build/Release/sourcemap.node]
 9: 0x7f4f742c8d98 Napi::ObjectWrap<SourceMapBinding>::InstanceVoidMethodCallbackWrapper(napi_env__*, napi_callback_info__*) [/usr/src/app/.tmp/parcel-v2/node_modules/@parcel/source-map/build/Release/sourcemap.node]
10: 0x9770e5  [/usr/local/bin/node]
11: 0xb8466c  [/usr/local/bin/node]
12: 0xb86477 v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) [/usr/local/bin/node]
13: 0x134e979  [/usr/local/bin/node]
Aborted (core dumped)

It didn't happen locally for me so far.

Maybe this happens because the Get() calls here

Napi::Value mapping = mappingsArray.Get(i);
Napi::Object mappingObject = mapping.As<Napi::Object>();
Napi::Object generated = mappingObject.Get("generated").As<Napi::Object>();
int generatedLine = generated.Get("line").As<Napi::Number>().Int32Value() - 1;
int generatedColumn = generated.Get("column").As<Napi::Number>().Int32Value();
Position generatedPosition = Position{generatedLine + lineOffset, generatedColumn + columnOffset};
Napi::Value originalPositionValue = mappingObject.Get("original");
if (originalPositionValue.IsObject()) {
Napi::Object originalPositionObject = originalPositionValue.As<Napi::Object>();
int originalLine = originalPositionObject.Get("line").As<Napi::Number>().Int32Value() - 1;
int originalColumn = originalPositionObject.Get("column").As<Napi::Number>().Int32Value();
Position originalPosition = Position{originalLine, originalColumn};
std::string sourceString = mappingObject.Get("source").As<Napi::String>().Utf8Value();

throw when the specified property doesn't exist.

You're probably right we should do a lot more checks here. I'm just scared it will impact performance too much at some point, the node values are already so slow.

If they do end up being a little slower I'll probably experiment with using vlq mappings everywhere in parcel no problem.

Maybe checking them on the JS side is faster?

@mischnic do you have a repro of this somewhere?
If not I'll just make one and turn it into a test

No, I only saw this in the benchmark logs and didn't investigate further.