containerd / nydus-snapshotter

A containerd snapshotter with data deduplication and lazy loading in P2P fashion
https://nydus.dev/
Apache License 2.0
172 stars 99 forks source link

cache: fix null pointer access when cachemanager.Disable is true #536

Closed jiangliu closed 1 year ago

jiangliu commented 1 year ago

Filesystem.cacheMgr is null when cacheManager.Disable is configured to be true, thus cause null pointer access. Fix it by always create the cacheMgr object no matter cacheManager.Disable is true or false.

codecov[bot] commented 1 year ago

Codecov Report

Merging #536 (fd4be45) into main (0da2966) will decrease coverage by 0.01%. Report is 1 commits behind head on main. The diff coverage is 0.00%.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/containerd/nydus-snapshotter/pull/536/graphs/tree.svg?width=650&height=150&src=pr&token=R3RX5WX6R1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containerd)](https://app.codecov.io/gh/containerd/nydus-snapshotter/pull/536?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containerd) ```diff @@ Coverage Diff @@ ## main #536 +/- ## ========================================== - Coverage 33.56% 33.56% -0.01% ========================================== Files 65 65 Lines 8175 8176 +1 ========================================== Hits 2744 2744 - Misses 5116 5117 +1 Partials 315 315 ``` | [Files Changed](https://app.codecov.io/gh/containerd/nydus-snapshotter/pull/536?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containerd) | Coverage Δ | | |---|---|---| | [snapshot/snapshot.go](https://app.codecov.io/gh/containerd/nydus-snapshotter/pull/536?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containerd#diff-c25hcHNob3Qvc25hcHNob3QuZ28=) | `0.00% <0.00%> (ø)` | |
sctb512 commented 1 year ago

Why not check if the cacheMgr is nil? It seems stargzResolver and referrerMgr also have the same problem.

jiangliu commented 1 year ago

Why not check if the cacheMgr is nil? It seems stargzResolver and referrerMgr also have the same problem.

With the evolution of nydus-snapshotter, cache manager becomes mandatory instead optional now. On the other hand, stargzResolver and referrerMgr are optional.