Skip to content

fix: Guard against race condition in FileStore::createCacheDir - #466

Open
Sarsharses wants to merge 1 commit into
ankitpokhrel:mainfrom
Sarsharses:fix/createcachedir-race
Open

fix: Guard against race condition in FileStore::createCacheDir#466
Sarsharses wants to merge 1 commit into
ankitpokhrel:mainfrom
Sarsharses:fix/createcachedir-race

Conversation

@Sarsharses

Copy link
Copy Markdown

Problem

Under concurrent uploads, FileStore::createCacheDir() can crash with mkdir(): File exists. On PHP setups that promote warnings to exceptions (e.g. Laravel's error handler), this surfaces as a fatal ErrorException on the first upload after the cache directory is absent.

Root cause

The check-then-create is not atomic:

if ( ! file_exists($this->cacheDir)) {
    mkdir($this->cacheDir);
}

Two concurrent requests can both pass file_exists(), then both call mkdir(); the loser gets a File exists warning.

Note: this is a different race from the cache-file data race fixed previously (#366, #368). Those hardened reads/writes of the cache file, whereas this fixes creation of the cache directory itself.

Fix

  • Suppress the race-loser warning with @mkdir(); the directory ends up existing either way.
  • Use is_dir() (the correct predicate, since file_exists() is also true for a file at that path).
  • Create recursively (0777, true) so nested cache paths work; default mode is unchanged.

Testing

Added it_creates_nested_cache_dir_recursively, which fails on main and passes with the fix. Full suite stays green.

A pure concurrency race can't be reproduced deterministically in a single-threaded test; the added test covers the recursive-creation half, and the warning suppression is the standard idiom used by Symfony/Laravel mkdir helpers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant