-
Notifications
You must be signed in to change notification settings - Fork 43
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
Fatal error, Check failed: result.second. #51
Comments
I'll try to narrow it down further tomorrow, but merely doing So either this isn't caused by |
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)? |
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 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). |
cc @Brooooooklyn do have an idea by chance? I'm not even sure how to debug this further |
@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 |
napi-rs/napi-rs#598 seems good for now. |
@Brooooooklyn Thanks! Yeah, I suspect that this problem doesn't really occur in isolation |
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. |
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! |
Should be fixed in v1.5.3. |
Thanks, it doesn't crash anymore, but now 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
instead of
|
Ugh, ok, I'll fix that. |
ok, fixed in v1.5.4. |
Thanks! Everything works now |
I've run into a situation where I consistently get this error (with macOS 10.15.5, Node v16.2.0)
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).The text was updated successfully, but these errors were encountered: