@diosmosis opened this Issue on January 6th 2015 Member

If multiple requests try to activate a plugin concurrently, OnDiskUIAsset::delete may throw. The situation that arises is as follows:

  1. one request starts the activation process, it checks whether the asset file exists
  2. another request checks whether it exists AND removes the file
  3. the original request tries to remove the file, but it doesn't exist anymore and an exception is thrown

To fix, OnDiskUIAsset should silence the unlink error and only throw if the file is not writable. This may require a bit of refactoring to the existing code.

@mnapoli commented on January 11th 2015 Contributor

How about something like this:

if (file_exists($file)) {
    $result = <a class='mention' href='https://github.com/unlink'>@unlink</a>($file);
    if (!$result && file_exists($file)) {
        throw new Exception(...);

i.e. check file_exists again after removing, that way if a concurrent request already removed the file, then the exception will not be thrown because the file will be gone anyway.

I also suggest we move those file handling methods to Piwik\Filesystem.

If you think that's a good solution I could work on a PR.

@diosmosis commented on January 12th 2015 Member

I think that will work. At least I can't think of a way for it to fail.

This Issue was closed on January 15th 2015
Powered by GitHub Issue Mirror