Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cache::put() throws a warning if the directory already exists #2518

Closed
janhartigan opened this issue Oct 18, 2013 · 8 comments
Closed

Cache::put() throws a warning if the directory already exists #2518

janhartigan opened this issue Oct 18, 2013 · 8 comments

Comments

@janhartigan
Copy link
Contributor

Due to my inability to convert a work project over to Laravel entirely, I've decided to use pieces of Illuminate in a CodeIgniter project. This has (as expected) worked wonderfully, but I seem to be running into a little problem with the cache file store. Consider this bit of code in the FileStore.php class (i.e. the put() and createCacheDirectory() methods:

public function put($key, $value, $minutes)
{
    $value = $this->expiration($minutes).serialize($value);

    $this->createCacheDirectory($path = $this->path($key));

    $this->files->put($path, $value);
}

protected function createCacheDirectory($path)
{
    try
    {
        $this->files->makeDirectory(dirname($path), 0777, true);
    }
    catch (\Exception $e)
    {
        //
    }
}

The first time this runs, it will create the application/cache/ac/bd/ directory (where ac and bd are the first bits of the md5 hash for the cache key) without any problems. However, once the cache expires (as it would, for example, when running Cache::remember()) and the file is deleted, or just when you call Cache::put() twice, you get this warning:

Warning: mkdir() [function.mkdir]: File exists in ...\vendor\illuminate\filesystem\Illuminate\Filesystem\Filesystem.php on line 296

I'm getting this issue in both Windows and Linux environments.

So I'm curious if this is expected behavior. Why would this only be happening outside of the Laravel context? What might I do to avoid this issue?

Thanks!

@JoostK
Copy link
Contributor

JoostK commented Oct 18, 2013

The Laravel framework converts all PHP errors to Exceptions, so it will be catched by the catch block.

This is a recent change (to circumvent a race condition) and before, it was in an if-statement checking for existence of the directory.

@janhartigan
Copy link
Contributor Author

Hmm that is interesting, thanks for the info @JoostK. So how interested are we in making these various components work on their own? My impression was that that was the nice thing about separating the core out into distinct packages. I can set up service providers to handle various inter-package dependencies, but this one is a bit tougher. I will probably end up converting warnings to exceptions (where is the code that does that, btw?), but it may not be the best solution for everyone.

@JoostK
Copy link
Contributor

JoostK commented Oct 18, 2013

I searched for the issue, it's #2379, and the commit was 4841dc8. @robclancy is having the same issue and I agree this is important to fix; the separation of packages is actually mainly to use them on their own.

@janhartigan
Copy link
Contributor Author

Nice find, @JoostK. I'm guessing the best way to fix this issue is to just undo that change, but I'll leave that to @taylorotwell

@taylorotwell
Copy link
Member

I'll look into fixing this.

@robclancy
Copy link
Contributor

For the record you can revert to using the 4.0.7 package for now and it will work fine as long as your project doesn't have massive amounts of hits and cause the race condition.

@taylorotwell
Copy link
Member

Fixed.

@janhartigan
Copy link
Contributor Author

Thanks, @taylorotwell! The fixing commit is here for future reference: 7bf5871

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

No branches or pull requests

4 participants