Skip to content

Commit 2d6c667

Browse files
author
Dale Sikkema
committed
MAGETWO-31834: Fatal error if trying to navigate to Frontend at first time after install
- Avoid race conditions by renaming generated file from temp file to real file - Disregard errors caused by generated file or directory already existing
1 parent 44e90c4 commit 2d6c667

File tree

6 files changed

+158
-246
lines changed

6 files changed

+158
-246
lines changed

lib/internal/Magento/Framework/Api/Test/Unit/Code/Generator/EntityChildTestAbstract.php

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,6 @@ public function testGenerate()
9191
->method('makeResultFileDirectory')
9292
->with($this->getResultClassName())
9393
->will($this->returnValue(true));
94-
$this->ioObjectMock->expects($this->once())
95-
->method('fileExists')
96-
->with($resultFileName)
97-
->will($this->returnValue(false));
9894

9995
//Mocking _generateCode call
10096
$this->classGenerator->expects($this->once())

lib/internal/Magento/Framework/Code/Generator/EntityAbstract.php

Lines changed: 13 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -247,52 +247,26 @@ protected function _validateData()
247247
{
248248
$sourceClassName = $this->getSourceClassName();
249249
$resultClassName = $this->_getResultClassName();
250-
$resultFileName = $this->_ioObject->getResultFileName($resultClassName);
251-
252-
// @todo the controller handling logic below must be removed when controllers become PSR-0 compliant
253-
$controllerSuffix = 'Controller';
254-
$pathParts = explode('_', $sourceClassName);
255-
if (strrpos(
256-
$sourceClassName,
257-
$controllerSuffix
258-
) === strlen(
259-
$sourceClassName
260-
) - strlen(
261-
$controllerSuffix
262-
) && isset(
263-
$pathParts[2]
264-
) && !in_array(
265-
$pathParts[2],
266-
['Block', 'Helper', 'Model']
267-
)
268-
) {
269-
$controllerPath = preg_replace(
270-
'/^([0-9A-Za-z]*)_([0-9A-Za-z]*)/',
271-
'\\1_\\2_controllers',
272-
$sourceClassName
273-
);
274-
$filePath = stream_resolve_include_path(str_replace('_', '/', $controllerPath) . '.php');
275-
$isSourceClassValid = !empty($filePath);
276-
} else {
277-
$isSourceClassValid = $this->definedClasses->classLoadable($sourceClassName);
278-
}
250+
$generationDir = $this->_ioObject->getGenerationDirectory();
251+
$resultDir = $this->_ioObject->getResultFileDirectory($resultClassName);
279252

280-
if (!$isSourceClassValid) {
253+
if (!$this->definedClasses->classLoadable($sourceClassName)) {
281254
$this->_addError('Source class ' . $sourceClassName . ' doesn\'t exist.');
282255
return false;
283256
} elseif ($this->definedClasses->classLoadable($resultClassName)) {
284257
$this->_addError('Result class ' . $resultClassName . ' already exists.');
285258
return false;
286-
} elseif (!$this->_ioObject->makeGenerationDirectory()) {
287-
$this->_addError('Can\'t create directory ' . $this->_ioObject->getGenerationDirectory() . '.');
288-
return false;
289-
} elseif (!$this->_ioObject->makeResultFileDirectory($resultClassName)) {
290-
$this->_addError(
291-
'Can\'t create directory ' . $this->_ioObject->getResultFileDirectory($resultClassName) . '.'
292-
);
259+
} elseif (
260+
!$this->_ioObject->makeGenerationDirectory()
261+
&& !$this->_ioObject->fileExists($generationDir)
262+
) {
263+
$this->_addError('Can\'t create directory ' . $generationDir . '.');
293264
return false;
294-
} elseif ($this->_ioObject->fileExists($resultFileName)) {
295-
$this->_addError('Result file ' . $resultFileName . ' already exists.');
265+
} elseif (
266+
!$this->_ioObject->makeResultFileDirectory($resultClassName)
267+
&& !$this->_ioObject->fileExists($resultDir)
268+
) {
269+
$this->_addError('Can\'t create directory ' . $resultDir . '.');
296270
return false;
297271
}
298272
return true;

lib/internal/Magento/Framework/Code/Generator/Io.php

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
*/
66
namespace Magento\Framework\Code\Generator;
77

8+
use Magento\Framework\Filesystem\FilesystemException;
9+
810
class Io
911
{
1012
/**
@@ -82,12 +84,37 @@ public function getResultFileName($className)
8284
/**
8385
* @param string $fileName
8486
* @param string $content
87+
* @throws FilesystemException
8588
* @return bool
8689
*/
8790
public function writeResultFile($fileName, $content)
8891
{
92+
/**
93+
* Rename is atomic on *nix systems, while file_put_contents is not. Writing to a
94+
* temporary file and renaming to the real location helps avoid race conditions on
95+
* some systems. Race condition can occur if the compiler has not been run, when
96+
* multiple processes are attempting to access the generated file simultaneously.
97+
*/
8998
$content = "<?php\n" . $content;
90-
return $this->filesystemDriver->filePutContents($fileName, $content);
99+
$rand = rand(0,1024);
100+
$tmpFile = $fileName . ".$rand";
101+
$this->filesystemDriver->filePutContents($tmpFile, $content);
102+
103+
try {
104+
$result = $this->filesystemDriver->rename($tmpFile, $fileName);
105+
} catch(FilesystemException $e) {
106+
if (!file_exists($fileName)) {
107+
throw $e;
108+
} else {
109+
/**
110+
* Due to race conditions, file may have already been written, causing rename to fail. As long as
111+
* the file exists, everything is okay.
112+
*/
113+
$result = true;
114+
}
115+
}
116+
117+
return $result;
91118
}
92119

93120
/**
@@ -138,7 +165,7 @@ private function _makeDirectory($directory)
138165
$this->filesystemDriver->createDirectory($directory, self::DIRECTORY_PERMISSION);
139166
}
140167
return true;
141-
} catch (\Magento\Framework\Filesystem\FilesystemException $e) {
168+
} catch (FilesystemException $e) {
142169
return false;
143170
}
144171
}

0 commit comments

Comments
 (0)