core/state, trie: fix memleak from fastcache

[tsahee]

Fastcache uses memory allocated by mmap, which is not garbage collected.
Data filled in the codeCache is therefore leaked when the stateDatabase is discarded (e.g. in StateAtBlock).
Same goes for cleans fastcache when used in the TrieDB.

Reset() doesn't unmap memory from the OS, but it does move all chunks to fastcache's global free chunk list, allowing it to be reused by later instances.

Fastcache is also used by the snapshot. I did not add Reset to those instances to minimize diff and because the snapshot is not regularly discarded. I can add cleaning that up as well if you wish.

[Ferrydh]

这是来自QQ邮箱的假期自动回复邮件。   您好，我最近正在休假中，无法亲自回复您的邮件。我将在假期结束后，尽快给您回复。

[rjl493456442]

I feel like it's unnecessary to use fastcache here as the codeCache. fastCache is mostly for caching large amount of data for GC-friendly purpose. Obviously not the case here.

[PlasmaPower]

Another option would be to maintain an in-house fastcache fork, which would have a finalizer on caches that frees them like you'd expect from Go being a garbage collected language. The upstream fastcache library is also unmaintained, e.g. VictoriaMetrics/fastcache#66 . I'd be happy to implement this but I want to make sure it makes sense first.

[karalabe]

Using finalizers is the wrong approach really, both in the library and in this code. The docs clearly state that Reset needs to be called. One option is to use the API correctly here, another is to use a caching library that does not need an explicit close mechanism.

[holiman]

One option is to use the API correctly here,

I looked into doing that -- calling Reset via a Close method on the database. It becomes a very large diff, touching a lot of tests, mainly.

I think we might be better off using a caching library which does not have it's own allocator. The code cache is a 64 MB cache (pretty tiny), we don't need a cache-on-steroids library for that little thing,

[rjl493456442]

@holiman totally agree

[holiman]

To follow up
bigcache has a different memory model -- and does not need Close(). It should be enough to just let the gc collect it. That said, it does have a Close method, and it is needed in case the config.CleanWindow > 0, because if that has been set then it will spin up a goroutine to do cache cleaning.

The fact that Close is not needed in most cases is an implementation detail, though, and it feels a bit wrong to just assume it will always be the case and rely on it.

[holiman]

Thanks for bringing this to our attention, but we will not merge this PR, since we don't think the solution is the best. We'll probably use #26092 to fix the problem instead.