node-lmdb icon indicating copy to clipboard operation
node-lmdb copied to clipboard

Error for " pointer being freed "

Open hanweifish opened this issue 7 years ago • 16 comments

Just simple getTxn, and get this error sometimes.

node(34473,0x10d5df5c0) malloc: *** error for object 0x102603250: pointer being freed was not allocated node(34473,0x10d5df5c0) malloc: *** set a breakpoint in malloc_error_break to debug

hanweifish avatar Mar 12 '19 00:03 hanweifish

Node: v8.12.0

hanweifish avatar Mar 12 '19 00:03 hanweifish

Can you please give me a test case with which you can reproduce the problem?

Venemo avatar Mar 12 '19 06:03 Venemo

It does not happened every time. operation is quite standard, like

try {
  this.dbi = this.LMDB.openDbi({
    name: DEFAULT_REPO,
    keyIsBuffer: true, // using buffer for enconding issue
    create: true, // will create new lmdb if not exist
  });
} catch (error) {
  throw new Error(`Error Set LMDB DBI. Error: ${error}`);
}

this.txn = this.LMDB.beginTxn();

const cursor = new lmdb.Cursor(this.txn, this.dbi);

// Close the database
this.dbi.close();
// Close the environment
this.LMDB.close();

have any thoughts about this kind of issue? memory leaking due to the usage of cursor?

hanweifish avatar Mar 12 '19 06:03 hanweifish

One more thing is I have this loop:

const cursor = new lmdb.Cursor(this.txn, this.dbi);
let locales = [];
for (let key = cursor.goToFirst(); key !== null; key = cursor.goToNext()) {
  const {local, keyString} = utils.abstractKey(key.toString('utf8'));
  if (!local || !keyString) {
    if (key.toString('utf8') === LOCALES_KEY) {
      const localesStr = this.txn.getBinary(this.dbi, key).toString('utf8');
      locales = utils.convertToArray(localesStr);
    }
    continue;
  }

  if (!translations[local]) {
    translations[local] = {};
  }
  translations[local][keyString] = this.txn
    .getBinary(this.dbi, key)
    .toString('utf8');
}

hanweifish avatar Mar 12 '19 06:03 hanweifish

@hanweifish I can't really tell what it is by just looking at that code fragment. Please give me a full test case which can reproduce the problem.

Venemo avatar Mar 12 '19 12:03 Venemo

this is the problem. It does not happened every time. But just happened randomly.

hanweifish avatar Mar 20 '19 20:03 hanweifish

@hanweifish All right, then please give me a full test case which can randomly reproduce the problem.

Venemo avatar Mar 21 '19 08:03 Venemo

I find the cause may from the missing "cursor.close()" in some code. My mistake, just ignore that. If possible, can you add this line in the Readme, on the part the "Example iteration over a database with a Cursor:"?, I think that may be help. Thanks for your time.

hanweifish avatar Mar 21 '19 15:03 hanweifish

@hanweifish So you're saying that the problem happens (randomly) when you don't close a cursor? If that is the case, I can look into that and also improve the readme.

Venemo avatar Mar 21 '19 16:03 Venemo

I think so. After I close the cursor, it tested overnight and works fine until now. (interval task to read the lmdb file)

hanweifish avatar Mar 21 '19 16:03 hanweifish

@hanweifish Okay. I'll look into it.

Venemo avatar Mar 21 '19 16:03 Venemo

Thanks. If you need more info, please let me know.

hanweifish avatar Mar 21 '19 16:03 hanweifish

@hanweifish It would still help if you could give me a working example so I can reproduce the problem. If you can't, I will try to remove cursor.close() from the indexing example and see if I can reproduce the crash that way.

Venemo avatar Mar 21 '19 16:03 Venemo

@Venemo I discovered this bug as well. Has something to do with increasing references to string values from the cursor key or .getCurrentString() outside of the cursor lifetime scope WHEN you forget to close the cursor. Example below. Run it a few times. The crash is intermittent.

const lmdb = require('node-lmdb');

env = new lmdb.Env();
env.open({
  path: "busted",
  mapSize: 2*1024*1024*1024, // maximum database size
  maxDbs: 1
});
db = env.openDbi({name: 'tests', create: true});
txn = env.beginTxn();
for(let i=1; i<100; i++) {
  txn.putString(db, "aaaaaaaa"+i, "bbbbbbbbbbbbbbbb"+i);
}
txn.commit();

txn = env.beginTxn();

function getRange(k1,k2, broken) {
  var c = new lmdb.Cursor(txn, db);
  const ret = {};
  for(let k = c.goToRange(k1); k < k2 ; k = c.goToNext()) {
    if(k) {
      ret[k] = c.getCurrentString();
    }
  }
  if(!broken) c.close();
  return ret;
}

//change boolean here to break/fix
//you may have to run several times - crash is intermittant 
ret = getRange('','z',true)
txn.commit();
console.log(ret);

jeffsenn avatar Aug 11 '21 21:08 jeffsenn

I believe that the LMDB docs indicate that a cursor using a write transaction must be closed before the transaction is committed (while it "still live": http://www.lmdb.tech/doc/group__mdb.html#gad685f5d73c052715c7bd859cc4c05188). If you don't explicitly close it before the transaction commits, either the cursor is never closed and leaks memory (worse case scenario), or it is closed during GC as node-lmdb does now, which causes a crash, which is both a reasonable expectation per the docs and a fail-fast error that is almost certainly preferable to the nightmare of a memory leak. I believe I have previously traced this error to the cursor close function (https://github.com/Venemo/node-lmdb/blob/master/src/cursor.cpp#L40), and it simply can't operate on a committed transaction.

So I don't think this is so much a bug, as it is part of the hazard of directly using cursors without ensuring (or using any mechanism to ensure) that the cursor must be closed prior to a commit.

kriszyp avatar Aug 11 '21 22:08 kriszyp

Can we maybe add some code that ensures all cursors are disposed when the transaction is committed?

Venemo avatar Aug 12 '21 05:08 Venemo