kriszyp / lmdb-js

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

C++ errors on windows with 1.6.0 #71

Closed vladar closed 2 years ago

vladar commented 2 years ago

I am getting some rather consistent crashes on windows:

image

When clicking ignore, see this:

image

and then this:

image

Surprisingly after going through those errors and "ignoring" them, node resumes working 🤷‍♂️

vladar commented 2 years ago

Got the same error on Ubuntu:

node: ../../nan/nan_object_wrap.h:109: virtual void Nan::ObjectWrap::Unref(): Assertion `!persistent().IsWeak()' failed.
Command terminated by signal 6
        Command being timed: "node --expose-gc node_modules/gatsby/dist/bin/gatsby.js build"
        User time (seconds): 676.34
        System time (seconds): 13.68
        Percent of CPU this job got: 126%
        Elapsed (wall clock) time (h:mm:ss or m:ss): 9:05.34
        Average shared text size (kbytes): 0
        Average unshared data size (kbytes): 0
        Average stack size (kbytes): 0
        Average total size (kbytes): 0
        Maximum resident set size (kbytes): 4000364
        Average resident set size (kbytes): 0
        Major (requiring I/O) page faults: 11
        Minor (reclaiming a frame) page faults: 3234663
        Voluntary context switches: 165214
        Involuntary context switches: 27211
        Swaps: 0
        File system inputs: 3136
        File system outputs: 6918176
        Socket messages sent: 0
        Socket messages received: 0
        Signals delivered: 0
        Page size (bytes): 4096
        Exit status: 0
error Command failed with exit code 134.

It is Node v14.17.3 and (v14.16.1 on Windows). One db has cache: true

kriszyp commented 2 years ago

Do you know if this is likely a new regression since one of the v1.6 betas? Generally all Unref() calls that lmdb-store makes are when closing dbs, cursors, etc. (or cleanup during GC). The most recent code change that was related to cleanup/closing was for closing environments when a context/thread shuts down (which actually should have also been in beta 3), but I am not sure how that would trigger extra Unref calls. Of course v1.6 had significant changes to cursors, but those were in all the betas too.

vladar commented 2 years ago

So I only used beta with tests and synthetic benchmarks. This error comes from a build of a production site with all its nuances. I am trying to bisect this error starting with 1.5.5. Will post any updates here.

kriszyp commented 2 years ago

Ok, I pushed a debug-unref branch with some additional debugging on Unref() calls, to see which one might be causing the problem, if you want to try that too.

vladar commented 2 years ago

Yeah, I don't see this problem with 1.5.5 (couldn't run with 1.6.0-betas on this site due to range issues).

Tried your branch, here is some additional output:

txn is already unref'ed from cursor close
node: ../node_modules/nan/nan_object_wrap.h:109: virtual void Nan::ObjectWrap::Unref(): Assertion `!persistent().IsWeak()' failed.
Command terminated by signal 6
vladar commented 2 years ago

Edit: nope, I removed those return() calls and still seeing this.

~I have only one idea: in some cases, we call return() method of range iterator directly. So basically our code calls this method:~

https://github.com/DoctorEvidence/lmdb-store/blob/ab09c0d87f9b40a686ac4b460795335156b48254/index.js#L903-L906

~That's because we have our own Iterable wrapper and when it exists early (e.g. with break from the for of loop) - we need to "close" the underlying iterator too (as described in this article).~

~Not sure if it can cause this problem.~

kriszyp commented 2 years ago

txn is already unref'ed from cursor close

Ah, great, I am pretty sure that points me to the problem. I think the referenced commit should probably fix it. Building a test for it is a little trickier, so I haven't fully verified it yet.

(And calling the return() method is perfectly normal and acceptable.)

vladar commented 2 years ago

FYI, I don't see this error anymore (with lmdb-store@1.6.1) 👍 Closing the issue.