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

[HttpKernel] Keep legacy container files for concurrent requests #25190

Merged
merged 1 commit into from Nov 29, 2017

Conversation

@nicolas-grekas
Copy link
Member

commented Nov 28, 2017

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #25177
License MIT
Doc PR -

Because concurrent requests might still be using them,
old container files should not be removed immediately,
but on a next dump of the container.

@nicolas-grekas nicolas-grekas force-pushed the nicolas-grekas:kernel-lazy-clear branch from c86504e to ee3b6fe Nov 28, 2017

@fabpot
fabpot approved these changes Nov 29, 2017
@fabpot

This comment has been minimized.

Copy link
Member

commented Nov 29, 2017

Thank you @nicolas-grekas.

@fabpot fabpot merged commit ee3b6fe into symfony:3.4 Nov 29, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details
fabpot added a commit that referenced this pull request Nov 29, 2017
bug #25190 [HttpKernel] Keep legacy container files for concurrent re…
…quests (nicolas-grekas)

This PR was merged into the 3.4 branch.

Discussion
----------

[HttpKernel] Keep legacy container files for concurrent requests

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #25177
| License       | MIT
| Doc PR        | -

Because concurrent requests might still be using them,
old container files should not be removed immediately,
but on a next dump of the container.

Commits
-------

ee3b6fe [HttpKernel] Keep legacy container files for concurrent requests

@nicolas-grekas nicolas-grekas deleted the nicolas-grekas:kernel-lazy-clear branch Nov 29, 2017

fabpot added a commit that referenced this pull request Nov 29, 2017
minor #25205 [HttpKernel] Fix race condition when clearing old contai…
…ners (nicolas-grekas)

This PR was merged into the 3.4 branch.

Discussion
----------

[HttpKernel] Fix race condition when clearing old containers

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Missed in #25190: when two concurrent requests create the new container concurrently, the last one would drop the old container files, because the first one just created the `*.legacyContainer` file.

Commits
-------

9d553f5 [HttpKernel] Fix race condition when clearing old containers
This was referenced Nov 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.