ino reuse in newInodeWithID (original) (raw)

At BuildBuddy, we run a modified version of the soci-snapshotter with the store/ directory from stargz-snapshotter patched in to support podman as described here. We recently encountered a bug caused by podman pulling two images in parallel in very quick succession (<1ms). I tracked it down to apparent ino reuse, hitting this error (note: the error message is incorrect, it should say refnode instead of rootnode, I can send a fix for this). Upon digging deeper, I discovered that the logic in idMap which attempts to reuse inos sometimes offers recently claimed inos for reuse. This is because the check for whether an inode is active calls Inode.Forgotten() which returns true for newly created, unused, non-persistent inodes (because lookupcount is 0 and parents is empty).

Practically, this means that if an refnode Inode has been created but not yet returned to the caller, and a second refnode.Lookup call attempts to mint a new ino, then the code will return the same ino for both of those Inodes (because the first one is releasable()), causing the fuse operations for the first Inode to route incorrectly, causing errors in stargz-snapshotter. You can tickle this bug by adding a time.Sleep() in newInodeWithID between when the Inode is created and when it is returned, and then getting that code to run twice in parallel by e.g. pulling images externally.

I think a reasonable fix is to only attempt to reuse inos after max loops all the way around, which should create a pretty big buffer time between when inos are initially allocated and when the snapshotter attempts to reuse them. In most cases I suspect that would prevent ino reuse, but maybe there are some cases where we're loop through all 4B inos. I'm open to other suggestions as well. Let me know what you think.