rely on object destructor to close db

[remicollet]

As explain in #36

PHP_VERSION : 7.3.22

=====================================================================
TEST RESULT SUMMARY
---------------------------------------------------------------------
Exts skipped    :    0
Exts tested     :   15
---------------------------------------------------------------------

Number of tests :   20                20
Tests skipped   :    0 (  0.0%) --------
Tests warned    :    0 (  0.0%) (  0.0%)
Tests failed    :    0 (  0.0%) (  0.0%)
Expected fail   :    0 (  0.0%) (  0.0%)
Tests passed    :   20 (100.0%) (100.0%)
---------------------------------------------------------------------
Time taken      :    5 seconds
=====================================================================

PHP_VERSION : 8.0.0beta4 (with pr #40)

=====================================================================
TEST RESULT SUMMARY
---------------------------------------------------------------------
Exts skipped    :    0
Exts tested     :   16
---------------------------------------------------------------------

Number of tests :   20                20
Tests skipped   :    0 (  0.0%) --------
Tests warned    :    0 (  0.0%) (  0.0%)
Tests failed    :    0 (  0.0%) (  0.0%)
Tests passed    :   20 (100.0%) (100.0%)
---------------------------------------------------------------------
Time taken      :    5 seconds
=====================================================================

[remicollet]

Perhaps LevelDBIterator::destroy()and LevelDBSnapshot::release() can also be deprecated.

[dktapps]

This looks good, thanks. I'd thought a few times about how to solve this issue but LevelDB->close() was an issue I couldn't make my mind up about.

I think this solution is usable, although it will have side effects - for example this code will no longer work correctly:

$db = new \LevelDB("somepath");
$db->close();
$db2 = new \LevelDB("somepath"); //LevelDBException thrown here

[remicollet]

Indeed this is a BC break, but the extension is still 0.2.1 (beta), so seems fine to change API in a 0.3.0 version.

And as "close" is deprecated it will raise user attention.

The other way will be to maintain a list of open iterator / snapshot for each DB, and close everything in the right order.

[remicollet]

Also notice than the current implementation of "close" generate segfaults (#36) or memory leaks, which is quite bad ;)

And the fix is trivial, $db->close() => unset($db)

Also benefit or new implementation:

function GenIterator($path) {
    $db = new LevelDB($leveldb_path);
    return (new LevelDBIterator($db));
}
foreach (GenIterator(...) as $k =$v) {
   ...
}

The $db will not be closed at end of the function, as referenced by the iterator.

[dktapps]

Yeah, the db recounting thing is kinda messed up, I don't know why it wasn't done this way to start with.

I think this is a reasonable way to solve the problem and I don't think that going the other way (keeping lists of snapshots / iterators) is worth the complexity.

I think it may also be possible to clean up these macros now, since LevelDBs won't be closed while an iterator is attached: https://github.com/reeze/php-leveldb/blob/master/leveldb.c#L41.

@reeze if you have no objections I think this is good to merge.

[remicollet]

Yeah, the db recounting thing is kinda messed up, I don't know why it wasn't done this way to start with.

Probably because writing library bindings with a 1/1 equivalent, while some procedural API are not useful in an OO API.