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

[5.6] Bugfix: concurrent write of bootstrap/cache/packages.php #25012

Merged
merged 1 commit into from
Jul 30, 2018

Conversation

bbiao
Copy link
Contributor

@bbiao bbiao commented Jul 30, 2018

When there is no bootstrap/cache/*.php, the php process will attemp to
build bootstrap/cache/packages.php, bootstrap/cache/services.php and
etc. If there are two or more php process doing the same thing, it is
possible for one of the process to read a dirty
bootstrap/cache/packages.php (not yet finish writing by another
process).

The solution add write and read lock to prevent read an dirty
packages.php and write a wrong services.php file.

When there is no bootstrap/cache/*.php, the php process will attemp to
build bootstrap/cache/packages.php, bootstrap/cache/services.php and
etc. If there are two or more php process doing the same thing, it is
possible for one of the process to read a *dirty*
bootstrap/cache/packages.php (not yet finish writing by another
process).

The solution add write and read lock to prevent read an *dirty*
packages.php and write a wrong services.php file.
@bbiao bbiao changed the title Bugfix: concurrent write of bootstrap/cache/packages.php [5.6] Bugfix: concurrent write of bootstrap/cache/packages.php Jul 30, 2018
@taylorotwell taylorotwell merged commit 683637a into laravel:5.6 Jul 30, 2018
@bbiao
Copy link
Contributor Author

bbiao commented Jul 31, 2018

@taylorotwell would you backport this to branch 5.5 ?

TheNodi added a commit to TheNodi/framework that referenced this pull request May 2, 2020
Filesystem get was introduced in laravel#25012 as a locking mechanism to prevent partial reads of the manifest file.
In laravel#26010 the locking was substituted by an atomic rename-based file write. The get called survived the refactor, resulting a noop file_get_contents that just trashes the output.

This commit removes the read entirely.
-
         return $this->manifest = file_exists($this->manifestPath) ?
             $this->files->getRequire($this->manifestPath) : [];
     }
TheNodi added a commit to TheNodi/framework that referenced this pull request May 2, 2020
Filesystem get was introduced in laravel#25012 as a locking mechanism to prevent partial reads of the manifest file.
In laravel#26010 the locking was substituted by an atomic rename-based file write. The get called survived the refactor, resulting a noop file_get_contents that just trashes the output.

This commit removes the read entirely.
taylorotwell pushed a commit that referenced this pull request May 2, 2020
Filesystem get was introduced in #25012 as a locking mechanism to prevent partial reads of the manifest file.
In #26010 the locking was substituted by an atomic rename-based file write. The get called survived the refactor, resulting a noop file_get_contents that just trashes the output.

This commit removes the read entirely.
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.

2 participants