Skip to content

[HttpKernel] lock when writting profiles #47435

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

Merged
merged 1 commit into from
Aug 31, 2022

Conversation

nicolas-grekas
Copy link
Member

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #47280
License MIT
Doc PR -

@DennisBirkholz
Copy link

I don't understand why there needs to be a 30+ lines change when a three line change is sufficient. Avoiding a lock if possible is always preferable over an unnecessary lock. The dump file is only ever written once and renaming a file on the same file system is atomic on all relevant file systems. This change touches two code points, which need to be kept in sync and the additional code increases the cognitive load when reasoning about this code in the future and will increase the barrier for new people to understand and contribute.

@nicolas-grekas
Copy link
Member Author

The reason is developer experience: with the atomic write approach, the race condition will lead to a "no-profile" state in the toolbar. With the lock, the profile will pause until the write is done and then load seamlessly.

The fact that the change is bigger is because I factorized some lines of code.

@fabpot
Copy link
Member

fabpot commented Aug 31, 2022

Thank you @nicolas-grekas.

@fabpot fabpot merged commit 5dfb353 into symfony:4.4 Aug 31, 2022
@nicolas-grekas nicolas-grekas deleted the hk-lock-profile branch August 31, 2022 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants