Use an exclusive lock when writing the cache file

[acinader]

fixes: #204

[acinader]

I suspect that there's a dogpile issue here where, when using the lock, several other requests could queue up to write the cache, so the file could get written multiple times. Accounting for that would require a different strategy to lock both read and write through the whole critical section.

Having said that, I'd prefer a dog pile on the file to a corrupt file that makes the site unusable.

[nikic]

This looks reasonable to me. We should probably also apply that to the 1.x branch.

[GrahamCampbell]

There is still a bug here, in that it is possible to require a partially written file. The file should maybe instead be written to disk under a different name, and then renamed to the the real file.

[nikic]

Yeah, that's a good point. I believes file renames on Linux at least are atomic.

[GrahamCampbell]

Yeh, I think so too.

[prisis]

Shouldnt it go a bit more in this direction https://github.com/webimpress/safe-writer/blob/master/src/FileWriter.php

[lcobucci]

Using rename() is similar to what I've implemented in https://github.com/nikic/FastRoute/pull/200/files#diff-cf865e6b333f1644ae9554f725b6813eR75-R92

[acinader]

The reading of a partially written cache file explains errors I saw in our logs. Good callout @GrahamCampbell.

I don't think that atomic rename is a good full solution. It does not address the cold cache, dogpile problem where more than one request is building the cache file simultaneously.

I think that php's flock can do what we really want which is block reads while the cache file is being calculated once. I can take stab at it if that sounds reasonable?

FYI: I'm using Slim 3 which depends on FastRoute.

[acinader]

fyi, i am still getting corrupted cache files.

i'll try and capture a corrupt one and post details to the issue

[lcobucci]

Closing this in favour #200 (will be backported to v1)