search

containerd / stargz-snapshotter

Fast container image distribution plugin with lazy pulling
https://github.com/containerd/containerd/issues/3731
Apache License 2.0
1.15k stars 114 forks source link

store: Avoid access to Forgotten() before configured #1599

Closed ktock closed 1 week ago

ktock commented 7 months ago

Fixes #1594

Our Lookup() implementation returns a newly created *fusefs.Inode to go-fuse. Then that *fusefs.Inode is configured and managed by go-fuse lib.

Though *fusefs.Inode.Forgotten() allows status check of that object, it shouldn't be called until that *fusefs.Inode is fully configured by go-fuse. Otherwize we hit issue like #1594.

To avoid this race, this commit adds a lock for avoiding calling *fusefs.Inode.Forgotten() during configuration on-going in go-fuse.

iain-macdonald commented 7 months ago

My understanding of the interactions between the go-fuse and stargz-snapshotter parts of the code may be off here, but won't the RLock/RUnlock calls in fsLocker surround the Lock/Unlock calls nested in newNodeWithID, causing deadlock? And if not, then isn't the lock in releasable insufficient as it doesn't guard the entire period from creation to initialization?

I think a simpler fix would be to used Inode.changeCounter to detect when an Inode hasn't been initialized and pair that with the Forgotten() check that's already present. I asked about this possibility in https://github.com/hanwen/go-fuse/issues/504, so what would you think about seeing what Han-Wen says and then coming up with a more elaborate solution if that doesn't work?

ktock commented 1 week ago

Fixed in #1808.