ContainerLoader, ContainerFactory: fixed thread-safety issues

Author: JanTvrdik

src/DI/ContainerFactory.php

@@ -110,44 +110,44 @@ private function loadClass()
 	{
 		$key = md5(serialize(array($this->config, $this->configFiles, $this->class, $this->parentClass)));
 		$file = "$this->tempDirectory/$key.php";
-
-		if (!$this->autoRebuild && (@include $file) !== FALSE) { // @ - file may not exist
+		if (!$this->isExpired($file) && (@include $file) !== FALSE) {
 			return;
 		}
 
 		$handle = fopen("$file.lock", 'c+');
-		if (!$handle) {
-			throw new Nette\IOException("Unable to open or create file '$file.lock'.");
+		if (!$handle || !flock($handle, LOCK_EX)) {
+			throw new Nette\IOException("Unable to acquire exclusive lock on '$file.lock'.");
 		}
 
-		if ($this->autoRebuild) {
-			flock($handle, LOCK_SH);
-			foreach ((array) @unserialize(file_get_contents("$file.meta")) as $f => $time) { // @ - file may not exist
-				if (@filemtime($f) !== $time) { // @ - stat may fail
-					@unlink($file); // @ - file may not exist
-					break;
+		if (!is_file($file) || $this->isExpired($file)) {
+			$this->dependencies = array();
+			$toWrite[$file] = $this->generateCode();
+			$files = $this->dependencies ? array_combine($this->dependencies, $this->dependencies) : array();
+			$toWrite["$file.meta"] = serialize(@array_map('filemtime', $files)); // @ - file may not exist
+
+			foreach ($toWrite as $name => $content) {
+				if (file_put_contents("$name.tmp", $content) !== strlen($content) || !rename("$name.tmp", $name)) {
+					@unlink("$name.tmp"); // @ - file may not exist
+					throw new Nette\IOException("Unable to create file '$name'.");
 				}
 			}
 		}
 
-		if (!is_file($file)) {
-			flock($handle, LOCK_EX);
-			if (!is_file($file)) {
-				$this->dependencies = array();
-				$code = $this->generateCode();
-				if (file_put_contents("$file.tmp", $code) !== strlen($code) || !rename("$file.tmp", $file)) {
-					@unlink("$file.tmp"); // @ - file may not be created
-					throw new Nette\IOException("Unable to create file '$file'.");
-				}
-				$tmp = array();
-				foreach ($this->dependencies as $f) {
-					$tmp[$f] = @filemtime($f); // @ - stat may fail
-				}
-				file_put_contents("$file.meta", serialize($tmp));
-			}
+		if ((@include $file) === FALSE) { // @ - error escalated to exception
+			throw new Nette\IOException("Unable to include '$file'.");
 		}
+		flock($handle, LOCK_UN);
+	}
+
 
-		require $file;
+	private function isExpired($file)
+	{
+		if ($this->autoRebuild) {
+			$meta = @unserialize(file_get_contents("$file.meta")); // @ - files may not exist
+			$files = $meta ? array_combine($tmp = array_keys($meta), $tmp) : array();
+			return $meta !== @array_map('filemtime', $files); // @ - files may not exist
+		}
+		return FALSE;
 	}
 
 

src/DI/ContainerLoader.php

@@ -61,46 +61,49 @@ public function getClassName($key)
 	private function loadFile($class, $generator)
 	{
 		$file = "$this->tempDirectory/$class.php";
-		if (!$this->autoRebuild && (@include $file) !== FALSE) { // @ - file may not exist
+		if (!$this->isExpired($file) && (@include $file) !== FALSE) {
 			return;
 		}
 
 		if (!is_dir($this->tempDirectory)) {
 			@mkdir($this->tempDirectory); // @ - directory may already exist
 		}
+
 		$handle = fopen("$file.lock", 'c+');
-		if (!$handle) {
-			throw new Nette\IOException("Unable to open or create file '$file.lock'.");
+		if (!$handle || !flock($handle, LOCK_EX)) {
+			throw new Nette\IOException("Unable to acquire exclusive lock on '$file.lock'.");
 		}
 
-		if ($this->autoRebuild) {
-			flock($handle, LOCK_SH);
-			foreach ((array) @unserialize(file_get_contents("$file.meta")) as $f => $time) { // @ - file may not exist
-				if (@filemtime($f) !== $time) { // @ - stat may fail
-					@unlink($file); // @ - file may not exist
-					break;
+		if (!is_file($file) || $this->isExpired($file)) {
+			list($code, $files) = call_user_func($generator, $class);
+
+			$files = $files ? array_combine($files, $files) : array();
+			$toWrite["$file.meta"] = serialize(@array_map('filemtime', $files)); // @ - file may not exist
+			$toWrite[$file] = "<?php\n$code";
+
+			foreach ($toWrite as $name => $content) {
+				if (file_put_contents("$name.tmp", $content) !== strlen($content) || !rename("$name.tmp", $name)) {
+					@unlink("$name.tmp"); // @ - file may not exist
+					throw new Nette\IOException("Unable to create file '$name'.");
 				}
 			}
 		}
 
-		if (!is_file($file)) {
-			flock($handle, LOCK_EX);
-			if (!is_file($file)) {
-				list($code, $dependencies) = call_user_func($generator, $class);
-				$code = "<?php\n" . $code;
-				if (file_put_contents("$file.tmp", $code) !== strlen($code) || !rename("$file.tmp", $file)) {
-					@unlink("$file.tmp"); // @ - file may not be created
-					throw new Nette\IOException("Unable to create file '$file'.");
-				}
-				$tmp = array();
-				foreach ((array) $dependencies as $f) {
-					$tmp[$f] = @filemtime($f); // @ - stat may fail
-				}
-				file_put_contents("$file.meta", serialize($tmp));
-			}
+		if ((@include $file) === FALSE) { // @ - error escalated to exception
+			throw new Nette\IOException("Unable to include '$file'.");
 		}
+		flock($handle, LOCK_UN);
+	}
+
 
-		require $file;
+	private function isExpired($file)
+	{
+		if ($this->autoRebuild) {
+			$meta = @unserialize(file_get_contents("$file.meta")); // @ - files may not exist
+			$files = $meta ? array_combine($tmp = array_keys($meta), $tmp) : array();
+			return $meta !== @array_map('filemtime', $files); // @ - files may not exist
+		}
+		return FALSE;
 	}
 
 }