kriszyp / lmdb-js

Simple, efficient, ultra-fast, scalable data store wrapper for LMDB
Other
486 stars 39 forks source link

Fatal error, Check failed: result.second. #51

Closed mischnic closed 3 years ago

mischnic commented 3 years ago

I've run into a situation where I consistently get this error (with macOS 10.15.5, Node v16.2.0)

#
# Fatal error in , line 0
# Check failed: result.second.
#
#
#
#FailureMessage Object: 0x700008408350
 1: 0x104ccaaae node::NodePlatform::GetStackTracePrinter()::$_3::__invoke() [/usr/local/Cellar/node/16.2.0/bin/node]
 2: 0x10551a21e V8_Fatal(char const*, ...) [/usr/local/Cellar/node/16.2.0/bin/node]
 3: 0x104ff1d25 v8::internal::GlobalBackingStoreRegistry::Register(std::__1::shared_ptr<v8::internal::BackingStore>) [/usr/local/Cellar/node/16.2.0/bin/node]
 4: 0x104db9f11 v8::ArrayBuffer::GetBackingStore() [/usr/local/Cellar/node/16.2.0/bin/node]
 5: 0x104c4e2b8 node::Buffer::Data(v8::Local<v8::Value>) [/usr/local/Cellar/node/16.2.0/bin/node]
 6: 0x1093963df EnvWrap::batchWrite(Nan::FunctionCallbackInfo<v8::Value> const&) [node_modules/lmdb-store/prebuilds/darwin-x64/node.abi93.node]
 7: 0x109397833 Nan::imp::FunctionCallbackWrapper(v8::FunctionCallbackInfo<v8::Value> const&) [node_modules/lmdb-store/prebuilds/darwin-x64/node.abi93.node]
 8: 0x104dff25e v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) [/usr/local/Cellar/node/16.2.0/bin/node]
 9: 0x104dfe92c v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) [/usr/local/Cellar/node/16.2.0/bin/node]
10: 0x104dfe003 v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments, v8::internal::Isolate*) [/usr/local/Cellar/node/16.2.0/bin/node]
11: 0x10542bb39 Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit [/usr/local/Cellar/node/16.2.0/bin/node]

There's a reproduction at https://github.com/mischnic/lmdb-store-issue-51 (see README for which commands to run), I was unfortunately unable to find a test case where it consistently crashes without Parcel being involved (yet!). Not sure if this is helpful to you. (I've set PARCEL_WORKERS=0, so everything runs on the main thread to eliminate concurrency bugs).

mischnic commented 3 years ago

I'll try to narrow it down further tomorrow, but merely doing console.log(bufferToBeWrittenToCache); before the failing call to lmdb here crashes Node.

So either this isn't caused by lmdb-store at all or this very buffer was actually previously read from lmdb and is somehow invalid

kriszyp commented 3 years ago

Yes, I noticed that even trying to log the buffer crashes Node as well. Do you know where these buffers (or Uint8Arrays) are originating from? I don't know if it is possible that they come from (what is supposed to be an internal call to) getBinaryUnsafe (than they wouldn't be safe to use a subsequent get)?

mischnic commented 3 years ago

This is getting even more confusing:

It crashes when adding this code

    if(asset.filePath.endsWith("node_modules/@babel/types/lib/ast-types/generated/index.js")) {
      process.stdout.write("1\n");
      process.stdout.write(compiledCode.length + "\n");
      process.stdout.write(compiledCode + "\n");
      process.stdout.write("2\n");
    }

at line 544 in https://unpkg.com/browse/@parcel/transformer-js@2.0.0-nightly.703/lib/JSTransformer.js .

The buffer compiledCode is returned from a Rust native module https://github.com/parcel-bundler/parcel/blob/6175fd73bb88dff9be47d4db31bc1c740959db04/packages/transformers/js/src/lib.rs#L89-L90

Either not using lmdb (and using our previous FS backend for the cache) or sending back a string instead of a buffer from Rust fixes this.

The only other thing that might be related about the buffer is that it is empty (length is 0). If I instead make it return a buffer representing a string with one space character, there's no error.

So my suspicion is there is something wrong in https://github.com/napi-rs/napi-rs (the Rust Node native module binding which returns that somehow invalid buffer) and/or lmdb-store (but it only crashes if both are used).

mischnic commented 3 years ago

cc @Brooooooklyn do have an idea by chance? I'm not even sure how to debug this further

Brooooooklyn commented 3 years ago

@mischnic I will add some test cases for this https://github.com/parcel-bundler/parcel/blob/6175fd73bb88dff9be47d4db31bc1c740959db04/packages/transformers/js/src/lib.rs#L88-L98 in napi-rs side and see how to find the root cause

Brooooooklyn commented 3 years ago

https://github.com/napi-rs/napi-rs/pull/598 seems good for now. It's late today, I will dig into this issue tomorrow

mischnic commented 3 years ago

@Brooooooklyn Thanks! Yeah, I suspect that this problem doesn't really occur in isolation

kriszyp commented 3 years ago

Ok, I figured this out, apparently V8 is giving this error when trying to access the pointer/address of a zero length buffer. I just needed to add a condition to not retrieve the address for a zero length buffer, as addressed in the above commit.

I am currently away from my main dev computer (on vacation), so might be a week or so before I get a full published version with pre-built binaries, but I could publish a version without binaries (it would need to compile), if that would help.

mischnic commented 3 years ago

Great! This error doesn't seems to occur frequently and if it does, we can workaround it by returning a buffer with one space character instead, so no need to hurry.

Enjoy your vacation!

kriszyp commented 3 years ago

Should be fixed in v1.5.3.

mischnic commented 3 years ago

Thanks, it doesn't crash anymore, but now store.get() returns "0" if that entry contains an empty buffer:

import * as lmdb from "lmdb-store";
let store = lmdb.open("cache", {
   name: "cache",
   encoding: "binary",
});

console.log(store.doesExist("x"));
console.log(store.get("x"));
await store.put("x", Buffer.alloc(0));
console.log(store.doesExist("x"));
console.log(store.get("x"));

prints

false
undefined
true
0

instead of

false
undefined
true
<Buffer >
kriszyp commented 3 years ago

Ugh, ok, I'll fix that.

kriszyp commented 3 years ago

ok, fixed in v1.5.4.

mischnic commented 3 years ago

Thanks! Everything works now