TYPO3.Flow

Hm, I had this in the pipeline: https://review.typo3.org/8786

Thanks! Aside from a simple issue with your code (forgot to throw a new exception), this seems fine. Two "theoretical" problems, though:

1. the unit tests fail, as we use fvsStream to mock the filesystem, but https://bugs.php.net/60232 breaks lock support for custom stream wrappers
2. Robert did not use LOCK_EX back in the days, because it is said to be useless as soon as you don't use a local filesystem (but NFS e.g.)

Any ideas about the former (except using the real FS for the tests) and any hard facts about the latter?

Patch set 1 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/23102

Patch set 2 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/23102

Patch set 2 for branch 2.0 has been pushed to the review server.
It is available at https://review.typo3.org/23081

There is still an issue with the current implementation (even with the patches applied): rename() issues a warning so

1$i = 0;
2$cacheEntryPathAndFilename = $this->cacheDirectory . $entryIdentifier . $this->cacheEntryFileExtension;
3while (($result = rename($temporaryCacheEntryPathAndFilename, $cacheEntryPathAndFilename)) === FALSE && $i < 5) {
4    $i++;
5}
6if ($result === FALSE) {
7    throw new \TYPO3\Flow\Cache\Exception('The cache file "' . $cacheEntryPathAndFilename . '" could not be written.', 1222361632);
8}

won't work as expected because a warning is issued for the first failed attempt leading to an exception in dev context. Besides this loop is probably iterated much too fast. We should add a usleep().

I would say that could be fixed by ignoring warnings from the rename (which we have to do anyway as Basti stated). An additional file_exists call is bad in terms of performance. And it doesn't give any guarantees since the calls to file_exists and rename are not atomic and race conditions could occur where the file existed but is gone when calling rename.

Mariusz Maroszczyk wrote:

The SimpleFileBackend "set" method also does not handle concurrency. "rename" should only be called if the file still exists (wasn't renamed yet).
A simple condition fixes the problem:
[...]

Patch set 3 for branch master of project Packages/TYPO3.Flow has been pushed to the review server.
It is available at https://review.typo3.org/23102