Investigate segfault

[ralphtheninja]

See #587 (comment)

[vweevers]

I'm able to replicate it, by increasing the number of iterators in the test to 100, setting { highWaterMark: 0 } (so that the iterator doesn't preemptively fetch all records) and forcing garbage collection before closing the database (which triggers a release of unused iterators). Trying to tweak it further.

[ralphtheninja]

I'm able to replicate it, by increasing the number of iterators in the test to 100, setting { highWaterMark: 0 } (so that the iterator doesn't preemptively fetch all records) and forcing garbage collection before closing the database (which triggers a release of unused iterators). Trying to tweak it further.

Nice!

[vweevers]

Jotting down some notes. There are 3 ways for an iterator to be ended, with subtle differences. Internally we have a std::map of active iterators, let's call it "the map".

1. iterator.end() (JS) which schedules EndWorker which:
   1. Calls Iterator#IteratorEnd
      • Destroys dbIterator_
      • Releases the snapshot
   2. Calls Iterator#Release
      • Removes iterator from the map and if map is empty, calls pending CloseWorker if any
2. db.close() (JS) which:
   1. Iterates the map and schedules EndWorker for each. Problem A: EndWorker can execute immediately, while we're still iterating, which means we're reading and writing from/to the map in parallel.
3. When garbage collected, which triggers the destructor of Iterator which:
   1. Calls Iterator#ReleaseTarget
   2. Cleans up slices etc
   3. NB. It does not release the snapshot or remove the iterator from the map.

I've refactored the code so that there's less code paths, but I only found Problem A after that, then fixed it with a mutex and got the tests to pass. Tomorrow I want to try and revert the refactorings, to see if Problem A is the only problem - or even a problem I introduced 😄

[vweevers]

I want to try and revert the refactorings, to see if Problem A is the only problem

It is not the only problem. I suspect that an iterator being in the map doesn't prevent it from getting garbage collected. So, any iterator that is not explicitly ended (and therefore stays in the map) and has no reference to it in JS-land, risks getting garbage collected, after which the map contains a null pointer (or something in that sense), which we hit on close.