If a process crashes half-way through writing a cache entry to a file cache, it will get corrupted

[itamarst]

For FileCache this may not be a problem insofar as deserializing will fail? For SeparateBodyFileCache it will be, insofar as it's just a pile-of-bytes for the data in the body file.

pip doesn't have this problem since it writes to a temporary file and then uses os.replace(). CacheControl may wish to adopt this.

[woodruffw]

pip doesn't have this problem since it writes to a temporary file and then uses os.replace(). CacheControl may wish to adopt this.

Makes sense. Would you be interested in sending a patch for this? Otherwise, I'll possibly have some time to look into this in the coming weeks (but no guarantees).

[itamarst]

The relevant code in pip is https://github.com/pypa/pip/blob/8a0f77c171d60344e6a3bf6e95ad5740c21575fd/src/pip/_internal/network/cache.py#L57, could probably just copy-paste the relevant utility code and integrate with the locking.

[Nikhil172913832]

Hi, is this issue still open? I can port the relevant atomic write utility from pip and integrate it with the existing locking, then submit a PR.

[woodruffw]

Hi @Nikhil172913832, it's still open but I think this was actually addressed a while back: #335 changed the file cache backend to use a temporary file + atomic rename. Can you reproduce that this is still a problem for you?

(In other words, it's open but that might just be because I forgot to close it. But if this is still something you can reproduce, I'd be happy to take a patch for it.)

[Nikhil172913832]

Thanks for the update. I’ll try to reproduce this on the current version and report back. If I can still reproduce it, I’ll submit a patch.

[Nikhil172913832]

Follow-up: I tried reproducing this on the current version but couldn’t reproduce the issue. It looks like it’s already been fixed.