Skip to content

catfile: Fix cache eviction potentially hanging

Patrick Steinhardt requested to merge pks-catfile-close-on-evict into master

When evicting catfile processes from the cache, then we first try to close the cached process via calling close(), and then cancel the process's context. Given that the command is created with SetupStdin, the command package will make sure to close stdin for us. But it's not closing stdout, which means that if the process has any data pending which it is trying to write to stdout, then it may hang forever.

In theory, only processes which are not dirty will ever be part of the catfile cache. But given that git-cat-file(1) always appends a newline to each object, we will also return processes to the cache which have exactly one byte pending, which is the expected newline. So it can definitely happen that a process ends up in the cache even though it's still got pending data, and this is even the default assumption. What's unclear is whether a process should in fact block in this write if it got a single byte pending -- but given that we use pipes to communicate with the process which are indeed synchronous, I can imagine that to happen. And we did see pipeline failures where eviction of the cache was hanging with a git-cat-file(1) process still lingering around.

Fix the potential deadlock by first cancelling the context. This will cause us to send a SIGKILL signal to the process, which should unblock it and thus also eviction of the cache.

Changelog: fixed

Closes #3854 (closed)

Merge request reports