fix race condition at mkdir (#16258) #16292
Conversation
ewgRa
commented
Oct 19, 2015
| Q | A |
|---|---|
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #16258 |
| License | MIT |
| Doc PR |
|
I'm not sure is IOException in this case broke BC. |
| @@ -42,7 +44,9 @@ public function __construct($dsn) | |||
| $this->folder = substr($dsn, 5); | |||
|
|
|||
| if (!is_dir($this->folder)) { | |||
| mkdir($this->folder, 0777, true); | |||
| if (false === @mkdir($this->folder, 0777, true) && !is_dir($this->folder)) { | |||
| throw new IOException(sprintf('Unable to create the storage directory (%s).', $this->folder)); | |||
There was a problem hiding this comment.
The HttpKernel should not need to know the Filesystem Component exceptions. Simply use \RuntimeException
|
Status: Needs Work |
|
@Tobion Thanks for review
Where I can read more? It is quite strange, HttpKernel work with disk, but throw RuntimeException instead of IOException. And what If I want to find all code that work with IO? With IOException it is more obvious. Status: Needs Review |
|
@ewgRa The HttpKernel doesn't have any dependency on the filesystem component, using an exception would require to add it. |
|
@jvasseur Thanks! Check composer at components and understand now that they can be used separately. Good to know. |
|
I've seen this also happening at https://github.com/symfony/symfony/blob/2.8/src/Symfony/Component/HttpKernel/HttpCache/Store.php#L40 Edit: Should that be fixed also? |
|
@epilgrim I think you are right. Checks added in HttpCache in three places where mkdir called. |
|
Done. Previous build seems have random hhvm test fail. |
|
(for ref.: #14728 (please ignore)) |
|
👍 should be merged in 2.3 as this is a bug fix. |
|
Thank you @ewgRa. |
This PR was submitted for the 2.8 branch but it was merged into the 2.3 branch instead (closes #16292). Discussion ---------- fix race condition at mkdir (#16258) | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #16258 | License | MIT | Doc PR | Commits ------- 2c2836c fix race condition at mkdir (#16258)
| */ | ||
| public function __construct($root) | ||
| { | ||
| $this->root = $root; | ||
| if (!is_dir($this->root)) { | ||
| mkdir($this->root, 0777, true); | ||
| if (false === @mkdir($this->root, 0777, true) && !is_dir($this->root)) { | ||
| throw new \RuntimeException(sprintf('Unable to create the store directory (%s).', $this->root)); |