[local storage] Remove IPC lock file after releasing the lock

[Sanjays2402]

Closes #1975

Description

fasteners.InterProcessLock creates its lock file but never removes it, and
LockLocalStorage.__exit__ did not remove it either. The file name is a sha256
of the locked path, so one file was left behind in the temp directory for every
distinct path ever locked -- a long running process writing many objects can
exhaust inodes. The lock file is now removed after the IPC lock is released.

Status

done, ready for review

Checklist (tick everything that applies)

• Code linting (required, can be done after the PR checks)
• Documentation
• Tests
• ICLA (required for bigger changes)

[micafer]

Removing the lock file immediately after releasing InterProcessLock introduces a race which can break mutual exclusion.

For example, process B may acquire the existing file lock immediately after process A releases it. A can then unlink the path, and process C can create and lock a new file at the same path. B and C would consequently hold locks on different inodes and could both enter the critical section.

The new test only verifies that the file is removed in a non-concurrent scenario, so it does not cover this race or verify that mutual exclusion is preserved.

I don't think the lock file can safely be removed as part of exit. We likely need a different bounded-lock or out-of-band cleanup strategy.

[Sanjays2402]

You're right, and the race is worse than the ordering I had in mind.

Even if I move the os.remove to before release() so the unlink happens while the lock is still held, it doesn't help: a process already blocked in acquire() is blocked on an fd for the old inode, so it wakes up holding a lock on an unlinked file while the next process creates a fresh file at the same path and locks that. Two holders, different inodes, exactly the scenario you described. There is no ordering of unlink and release inside __exit__ that avoids it, because the identity of the lock is the inode and the lookup key is the path.

The only ways I can see to make it safe are all heavier than the leak deserves:

• a second, permanent lock file guarding create+unlink of the real one, which just moves the leak to the guard file (one per path, same cardinality);
• refcounting holders in the file itself, which needs its own atomicity story across processes;
• out-of-band cleanup (age-based sweep of <tmp>/lock/ at driver init or a maintenance entry point), which is safe but is a different feature from what this PR claims.

And the thing being leaked is a zero-byte file per distinct locked path, bounded by the set of paths the process touches, in a directory the OS already reclaims. That's a poor trade against any risk to mutual exclusion.

So I'll concede this one — the approach in the PR is not fixable in place. Happy to close it. If you think the age-based sweep is worth having as a separate change I can open that instead, but I'd rather you say so than have another speculative PR land in your queue.