kriszyp / lmdb-js

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

Fatal crash after db.get #173

Closed oldrich-s closed 2 years ago

oldrich-s commented 2 years ago

We have just updated lmdb.js from version 2.1.7 to version 2.4.3 on Windows Server 2016 x64, NodeJS v16.13.0.

For one particular key, sometimes when we do db.get('key') where the returned value is a string, the whole process crashes with the following error.

I have traced it to the place where lmdb.js v2.1.7 works whereas lmdb v2.2.0-beta-1 crashes.

Do you have any idea what could go wrong?

022-05-25T07:39:53.237Z

#
# Fatal error in , line 0
# Check failed: result.second.
#
#
#
#FailureMessage Object: 000000364156D610

2022-05-25T07:39:53.570Z
 1: 00007FF76228013F v8::internal::CodeObjectRegistry::~CodeObjectRegistry+112495
 2: 00007FF76219D3CF v8::CFunctionInfo::HasOptions+7055
 3: 00007FF762E73202 V8_Fatal+162
 4: 00007FF7628E436D v8::internal::BackingStore::Reallocate+637
 5: 00007FF762B2D869 v8::ArrayBuffer::GetBackingStore+137
 6: 00007FF762253ED9 napi_get_typedarray_info+393
 7: 00007FFA191917E2 
 8: 00007FF76224E7C7 node::Stop+36391
 9: 00007FF762B63ACF v8::internal::SetupIsolateDelegate::SetupHeap+53823
10: 000001D8EB694EC0 
kriszyp commented 2 years ago

I have ideas, we have seen this type of error before (#161 , which was actually traced back to an issue with napi-rs), but not sure of the path to this one. A few questions:

oldrich-s commented 2 years ago

I use no compression, the value is bigger than 32kb, I am using just plain get and it is plain string value using MessagePack.

I think the error occurs when I have two separate instances of lmdb in the same nodejs process connected to the same database and both open at the same time. Then the first instance reads many values from the database and it goes fine. Then the second lmdb instance reads the exactly same values as the first instance and there it crashes.

To make it even more complex both the first and second instance of lmdb are obtained by on the fly compilation of the same javascript file using "new Module" nodejs functionality.

Meaning, I have a javascript code that imports lmdb and does the database reading. I compile the code by Module library and execute it. Then later I compile the same code again and run it and there it crashes

kriszyp commented 2 years ago

Ok, that makes sense then. V8 crashes in this way when two external (and attached) buffers are created that point to the same address, and usually lmdb-js detaches any previous attached buffer before creating a new one to avoid this, but with two instances, it certainly is possible for each to have a buffer pointing to the same address. There are actually a number of problems that can occur when two lmdb instances in the same process point to the same database, so lmdb-js tries hard to detect this.

Forgive me for my ignorance, by "new Module" are you referring to the functionality from node's vm module? Currently, lmdb-js sets a global variable __lmdb_envs__ so that a second lmdb instance can detect other library instances. But if you have separate vm contexts, I assume that global would be hidden from a second context, right?

oldrich-s commented 2 years ago

I am referring to nodejs native module:

https://nodejs.org/docs/latest-v16.x/api/module.html#the-module-object

it can be constructed even when the documentation does not state it - to mimic require without using the require function.

So both instances should have access to the same global variable.

Also it is most likely a regression introduced between v2.1.7 and 2.2beta.

I will try to look at the commits between the two tags to see if I can spot something

kriszyp commented 2 years ago

Well, there was definitely new functionality introduced to use external buffers for larger values, so I am pretty sure I know what the issue is and where to fix it. And if both lmdb-js instances do have access to same global variable, I can certainly use this to ensure external buffers are detached properly between get calls across lmdb-js instances. I'll try to get a fix out today, and we can see if that addresses your issue.

kriszyp commented 2 years ago

I published a v2.4.4 with a fix that hopefully should address this issue.

I do actually intend to do more with extending the use of external zero-copy buffers in a future release, but will try to test this scenario for that.

oldrich-s commented 2 years ago

Thanks a lot 👍. I will test it out on monday 😉.

oldrich-s commented 2 years ago

Hi, so far it looks like it fixed the issue. Thanks a lot 👍.