chjj / liburkel

Authenticated key-value store (i.e. an urkel tree)
Other
315 stars 12 forks source link

LeakSanitizer reports leaks #7

Closed pepyakin closed 4 years ago

pepyakin commented 4 years ago

During fuzzing I am observing a steady climb of rss up to eventual OOM kill.

The leak looks like this

==128517==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 136 byte(s) in 1 object(s) allocated from:
    #0 0x560dd6e547ad in malloc /rustc/llvm/src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
    #1 0x560dd6ff5204 in urkel_store_get_history (/home/lilpep/dev/urkel-rs/fuzz/target/x86_64-unknown-linux-gnu/release/exercise_api+0x2b0204)

SUMMARY: AddressSanitizer: 136 byte(s) leaked in 1 allocation(s).
chjj commented 4 years ago

Hmm. I just noticed urkel_tx_inject probably has a leak: https://github.com/chjj/liburkel/blob/master/src/tree.c#L937

Any chance the fuzzer is calling that?

chjj commented 4 years ago

Another leak fixed in urkel_tx_clear. This is what I get for porting from a GC'd language I guess.

pepyakin commented 4 years ago

Any chance the fuzzer is calling that?

yup, I think I cover most of the API (excluding those functions that duplicate cursor)

pepyakin commented 4 years ago

with the latest commit the memory consumption is steady and there are no reports from LeakSanitizer. That is, on linux. On macOS there are also no reports, but for some reason still RSS climbs : ?

chjj commented 4 years ago

@pepyakin very strange that apple produces different results than linux. That makes me think this has something to do with an OS call, but I can't think of anything that would cause this.

Aside from that, the memory could climb (but not leak) for a few reasons I can think of:

  1. Urkel caches all root hashes in memory. If there are millions of commits, there will be millions of root hashes stored in a hash table in memory. These are small, but if there are tons of commits I suppose memory could grow.
  2. If you're doing giant writes, the write buffer will grow non-linearly. It should be capped around 64mb-128mb though.
  3. Transaction state is held in memory and will only be free'd once committed. If you're inserting millions of records into a transaction, those will all be held in memory until the tx is committed/injected/cleared/destroyed.

Though these same things would be happening on linux if any of them were the case. I'll try to experiment with an apple machine.

pepyakin commented 4 years ago

ok, nevermind, I think that was some glitch on my side. The growth is there but it is rather negligable. The leak sanitzer doesn't complain at least. Closing.

pepyakin commented 4 years ago

Oh, you beat me to it.

Actually I don't think that the fuzzed api calls are giant or anything. The value buffers are limited to 1024 and there is rather limited sequence of calls (I would guess that it's less than 100). After each round a new temporary prefix is generated. Moreover, during one round there might be a full flush - i.e. close the db and reopen it.

Assuming that those caches do not live in some thread locals or globals I think it shouldn't affect the RSS anyhow.