Fix chained batch segfault

binding.cc

@@ -1773,21 +1773,30 @@ NAPI_METHOD(batch_clear) {
  */
 struct BatchWriteWorker final : public PriorityWorker {
   BatchWriteWorker (napi_env env,
+                    napi_value thisObj,
                     Batch* batch,
                     napi_value callback,
                     bool sync)
     : PriorityWorker(env, batch->database_, callback, "leveldown.batch.write"),
       batch_(batch),
-      sync_(sync) {}
+      sync_(sync) {
+        // Prevent GC of batch object before we execute
+        NAPI_STATUS_THROWS(napi_create_reference(env_, thisObj, 1, &thisRef_));
+      }
 
-  ~BatchWriteWorker () {}
+  ~BatchWriteWorker () {
+    napi_delete_reference(env_, thisRef_);
+  }
 
   void DoExecute () override {
     SetStatus(batch_->Write(sync_));
   }
 
   Batch* batch_;
   bool sync_;
+
+private:
+  napi_ref thisRef_;
 };
 
 /**
@@ -1801,7 +1810,7 @@ NAPI_METHOD(batch_write) {
   bool sync = BooleanProperty(env, options, "sync", false);
   napi_value callback = argv[2];
 
-  BatchWriteWorker* worker  = new BatchWriteWorker(env, batch, callback, sync);
+  BatchWriteWorker* worker  = new BatchWriteWorker(env, argv[0], batch, callback, sync);
   worker->Queue();
 
   NAPI_RETURN_UNDEFINED();

package.json

@@ -6,8 +6,8 @@
   "main": "leveldown.js",
   "scripts": {
     "install": "node-gyp-build",
-    "test": "standard && nyc tape test/*-test.js",
-    "test-gc": "npx -n=--expose-gc tape test/{cleanup,iterator-gc}*-test.js",
+    "test": "standard && npm run test-gc && nyc tape test/*-test.js",
+    "test-gc": "npx -n=--expose-gc tape test/{cleanup,iterator-gc,chained-batch-gc}*-test.js",
     "test-electron": "electron test/electron.js",
     "coverage": "nyc report --reporter=text-lcov | coveralls",
     "rebuild": "node-gyp rebuild",

test/chained-batch-gc-test.js

@@ -0,0 +1,43 @@
+'use strict'
+
+const test = require('tape')
+const testCommon = require('./common')
+
+function repeat (name, fn) {
+  for (let i = 0; i < 100; i++) {
+    test(`${name} (${i})`, fn)
+  }
+}
+
+// When we have a chained batch object without a reference, V8 might GC it
+// before we get a chance to (asynchronously) write the batch.
+repeat('chained batch without ref does not get GCed before write', function (t) {
+  t.plan(2)
+
+  const db = testCommon.factory()
+
+  db.open(function (err) {
+    t.ifError(err, 'no open error')
+
+    let batch = db.batch()
+
+    for (let i = 0; i < 1e5; i++) {
+      batch.put(String(i), 'value')
+    }
+
+    // The sync option makes the operation slower and thus more likely to
+    // cause a segfault (if the batch were to be GC-ed before it is written).
+    batch.write({ sync: true }, function (err) {
+      t.ifError(err, 'no error from write()')
+    })
+
+    // Remove reference
+    batch = null
+
+    if (global.gc) {
+      // This is the reliable way to trigger GC (and the bug if it exists).
+      // Useful for manual testing with "node --expose-gc".
+      global.gc()
+    }
+  })
+})