Fix memory leak when calling get_writebatch and avoid unnecessary clones #786
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Memory leak
It seems
wi
is not deallocated properly which causes a memory leak when callingget_writebatch
.rocksdb_transaction_get_writebatch_wi allocates memory with
malloc
, which never gets deallocated.There is rocksdb_writebatch_wi_destroy but that tries to delete the memory allocated with malloc which is UB and tries to clear
rep
. Usingrocksdb_free
seems to be the right choice.Extra allocations
There also seems to be unnecessary cloning of the serialized writebatch. The code clones the data once by calling
to_owned
on the slice but the data is also cloned here:https://github.com/facebook/rocksdb/blob/main/db/c.cc#L1905
To improve that, I think we can just pass the
ptr
directly torocksdb_writebatch_create_from