Warning: mkdir(): File exists [...] src/includes/traits/Plugin/InstallUtils.php on line 356

[raamdev]

Originally reported in the community forum, and here.

Warning: mkdir(): File exists in /home/www/wp-content/plugins/comet-cache/src/includes/traits/Plugin/InstallUtils.php on line 356

Here's src/includes/traits/Plugin/InstallUtils.php#L356.

---

Quoting private discussion from Slack.

@raamdev writes...

How do you feel about suppressing Warnings on this line: https://github.com/websharks/comet-cache/blob/160521/src/includes/traits/Plugin/InstallUtils.php#L356

Related support thread where some users, who unknowingly have PHP display_errors = On, are seeing a warning occasionally show up on the front-end of their site: https://wordpress.org/support/topic/php-warning-just-showed-up-on-my-published-site

WordPress seems to suppress such mkdir() warnings in the WordPress Filesystem Class: https://github.com/WordPress/WordPress/blob/4.5/wp-admin/includes/class-wp-filesystem-direct.php#L434

@jaswsinc writes...

Suppressing that warning... Well, I think it depends on the context in which a call to mkdir() is being made, and also on the arguments being passed to mkdir(). In the line you referenced in the WP Filesystem class, it's a function that returns a value true or false so a caller can test for failure. The warning is suppressed there for a good reason. A caller may try to create something and ​_expect_​ a possible failure, in which case no warning should be triggered. The caller can decide what to do.

As for the arguments, the big one would be the -p flag in the last argument. If that's true and all parents should be created too, then a warning becomes slightly more appropriate.

So looking at the context and arguments we are using in Comet Cache, I'm seeing that a call to mkdir() here looks like an attempt to rectify a rather strange case where the /cache directory does not exist. Normally, that should not occur, because the cache directory is created when the plugin is installed and unless I'm mistaken it's not even deleted when we do a full wipe. Only when the plugin is uninstalled is that directory removed.

Of course, it could be removed by a site owner in some way by mistake, or it may not exist after an automated deployment of some sort.

Looking just above that line I'm seeing a similar failure where we just return false and do nothing instead of failing hard with a warning. So I suppose we could do the same for the mkdir() call also. Something like @mkdir(...); return false; .

Having said that, if there's anything we can do to diagnose this I'd be curious to know why that's happening. It seems like there is another problem on the site if that call fails.

@raamdev writes...

Looking just above that line I'm seeing a similar failure where we just return false and do nothing instead of failing hard with a warning. So I suppose we could do the same for the mkdir() call also. Something like @mkdir(...); return false;

Yeah, that sounds like a good idea to me. We also check if !is_dir($cache_dir) again a few lines after that, so we have some redundancy in there to make sure that the cache directory exists and is writable before we return true.

@jaswsinc writes...

I'd try to look closer at this and see if we can figure out why that might be occurring. If you're correct, then maybe it has something to do with the stat cache and we can prevent that scenario from occurring altogether by running clearstatcache() in PHP in more places than we do currently.

PHP is supposed to maintain the stat cache for us automatically. More recent versions of PHP have improved in this regard too. However, there have been a few cases over the years where I have noticed that some functions do not handle this properly. For instance, in older PHP versions < 5.3 it was possible to unlink() a file and the stat cache was ​_NOT_​ cleared implicitly at all times. I had to do it by calling clearstatcache() explicitly.

To illustrate that point...

if (is_file($file)) {
    unlink($file);
}
if (is_file($file)) {
  // true? ... nope, gotta clear the stat cache so PHP knows what just happened.
}

So I'd start by doing a review of this article at PHP.net to see if anyone has documented anything recently that is related to this sort of behavior.
http://php.net/manual/en/function.clearstatcache.php

Also do a review of any functions in Comet Cache that deal with file deletions, renames, etc. Anything that might trick PHP into thinking the cache directory does not exist when it does. Or, you can also look at functions that deal with directory creation and see if there is something that tricks PHP into thinking it doesn't exist yet when it does, because we created the directory in the same process.

[raamdev]

Noting that I also requested PHP version info and phpinfo() from the two users who originally reported the issue. I'll update here if/when I hear back.

[raamdev]

@jaswsinc writes...

PHP is supposed to maintain the stat cache for us automatically.

Right, but if a system call modifies things outside of PHP, the stat cache can become outdated; see this comment: http://php.net/manual/en/function.clearstatcache.php#116554

I also came across this interesting comment about why using is_dir() is not the best way to check if a directory exists before creating it:

This is problematic on unix systems because everything is a file, even directories. If a file exists with the same name, said directory cannot be added. So, simply checking if a "file" is a directory isn't sufficient. Instead you should check if(!file_exists($path)).

Here's what we're currently doing in Comet Cache:

        if (!is_dir($cache_dir)) {
            mkdir($cache_dir, 0775, true);
        }

So, if $cache_dir is, for some reason, a file instead of a directory, that conditional will pass and Comet Cache will attempt to create the directory, but the mkdir() call will fail with a warning (because the "file exists").

---

@jaswsinc Here's what I propose we do to make that section of code a bit more robust:

        clearstatcache();
        if (!file_exists($cache_dir)) {
            mkdir($cache_dir, 0775, true);
        }

[jaswrks]

That looks good to me also.