Race Condition in Zend\Cache\Storage\Adapter\Filesystem::prepareDirectoryStructure

[dasann]

In case there are lot of parallel requests writing caches in the same directory, you may have problems at creating the cachedir.
If a parallel request creates the directory between

if (file_exists($pathname)) {
            return;
}

and

if (!$res) {
                $oct = ($perm === false) ? '777' : decoct($perm);
                $err = ErrorHandler::stop();
                throw new Exception\RuntimeException(
                    "mkdir('{$pathname}', 0{$oct}, true) failed", 0, $err
                );
}

the application will throw an exception.

The solution could be just to add another file_exist($pathname) to the second condition, like this:

 if (!$res && !file_exists($pathname)

I also wrote a little quick and dirty test to verify the problem and the solution:

public function testRaceConditionInPrepareDirectoryStructure()
    {
        $cachedirPath = 'cachetest';

        \vfsStreamWrapper::register();
        $root = \vfsStream::newDirectory($cachedirPath);
        \vfsStreamWrapper::setRoot($root);

        $builtInWrapperMock = LouisMockery::mock('Zend\Cache\Storage\Adapter\BuiltInWrapper');
        $builtInWrapperMock->shouldReceive('fileExists')->andReturn(false);

        $storageAdapter = new \Zend\Cache\Storage\Adapter\Filesystem();

        $storage = new \ReflectionClass('Zend\Cache\Storage\Adapter\Filesystem');
        $storage->getMethod('prepareDirectoryStructure')->setAccessible(true);

        $storage->getMethod('setBuiltInWrapper')->invoke($storageAdapter, $builtInWrapperMock);
        $method = $storage->getMethod('prepareDirectoryStructure');
        $method->setAccessible(true);
        $method->invoke($storageAdapter, \vfsStream::url($cachedirPath));
    } 

Because it's difficult to mock built-in-functions, I wrapped the file_exist() in a special class called BuiltInWrapper.

[marc-mabe]

@david-sann You are right. In this case the exception is wrong.
For the mocked filesystem part of testing it needs more work but this is another example why it could be a good idea.

[Ocramius]

Handled in #6573