7

At work, I've inherited a web application that has a file upload process. Part of this process occasionally (once every two weeks or so) triggers the following error:

PHP Warning:  mkdir(): File exists in {{{file_path_name_redacted}}} on line 7

Looking at lines 6-8, gives us:

if(!is_dir($storeFolder)){
    mkdir($storeFolder, 0644, TRUE);
}

Given this file can be hit by multiple PHP processes, I believe race conditions may be coming into play here. I've seen the same problem on other sites that I've managed in the past, similarly occurring only once in a blue moon.

What I believe is happening is that users are double-clicking on the upload button, which causes two PHP processes to execute at almost exactly the same time, like this:

Process 1 executes line 6 - dir does not exist
Process 2 executes line 6 - dir does not exist
Process 1 executes line 7 - directory is created
Process 2 executes line 7 - directory cannot be created as it already exists

Is this a case of race conditions, as I explained above (i.e. has anyone else noticed this), and/or is there some way of mitigating the error other turning off error reporting for warnings?

e_i_pi
  • 4,590
  • 4
  • 27
  • 45
  • 1
    Might be worthwhile checking `file_exists()` as well, otherwise if `$storeFolder` exists as a file (rather than a directory), then `is_dir()` willl validly return false, but `mkdir()` will fail – Mark Baker Jun 02 '17 at 07:10
  • I'll try that, but the artifacts in the parent folder are only ever created by this process. i.e. only directories are ever in that folder – e_i_pi Jun 03 '17 at 06:19

2 Answers2

6

Php inspections confirms that a race condition exists, and suggests that the safest way to write your code is:

if (!is_dir($dir) && !mkdir($dir) && !is_dir($dir)) {
        throw new \RuntimeException(sprintf('Directory "%s" could not be created', $dir));
    }

A bit more of explanation

It feels strange, but it certainly works. Best of luck.

Hugues D
  • 322
  • 2
  • 10
  • I'm not in a position to test this any more, as I've left my previous workplace, but the links you've provided explain the situation precisely. I wouldn't be surprised if this works, as `is_dir` and `mk_dir` may call different processes that are queued up differently. Thanks for your answer! – e_i_pi Sep 15 '19 at 09:04
1

I have seen many projects use Hugues D's solution, which seems to work quite well. However, it can fail when using $recursive = true, because of a bug in PHP reported in 2005, which they somehow refuse to fix (yes, it is a bug).

Here's a snipped that has served me well so far:

/**
 * Safer version of mkdir(..., ..., true) without race condition issues
 *
 * @see https://bugs.php.net/bug.php?id=35326
 *
 * @param string $dirname A directory path to create
 * @param int    $mode    Permission
 */
function safeMkdirRecursive(string $dirname, int $mode = 0777): void {
    $current = '';
    foreach (explode(DIRECTORY_SEPARATOR, $dirname) as $part) {
        $current .= $part;
        if ($current !== '' && !@mkdir($current, $mode) && !is_dir($current)) {
            throw new RuntimeException('Failed to create directory: ' . $current);
        }
        $current .= DIRECTORY_SEPARATOR;
    }
}

Disclaimer: I have not tested this on Windows!

jlh
  • 4,349
  • 40
  • 45