[HttpFoundation] Use locking for file_put_contents in MockFileSessionStorage #39167
Conversation
src/Symfony/Component/HttpFoundation/Session/Storage/MockFileSessionStorage.php
Show resolved
Hide resolved
|
I am afraid the So, would you (maybe @jderusse can tell) endorse using |
|
Further failed test runs on our CI also indicate that there may be a race condition related to the |
I don't think that we should introduce such a dependency in a bugfix release, but you could certainly borrow the logic from there. |
|
|
|
That would give us atomic writes, but what about race conditions and/or concurrent session access? Last time I checked, PHP’s built in session functions used locking: When one process read the session data on Might be too much hassle for something with |
then we should replace the call to file_get_contents() by a proper fopen+flock
Yes, and in theory also you can get valid yet different-than-expected content when a concurrent process writes new content. Maybe you should use separate folders to store the session in your case?
yes, but that's too much work, and many real drivers don't have locking either. |
|
@mpdude Are you still interested in moving this PR forward? |
|
@fabpot Yes, only somewhat limited in spare time atm. |
|
I'm facing the same thing in my functional tests (worst debugging time of my life). I just tried the |
|
Please check #39167 and confirm it fixes your issue if you can. |
…ge (nicolas-grekas) This PR was merged into the 4.4 branch. Discussion ---------- [HttpFoundation] use atomic writes in MockFileSessionStorage | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #39167 | License | MIT | Doc PR | - Instead of #39808 Commits ------- 5290e97 [HttpFoundation] use atomic writes in MockFileSessionStorage
I am running tests with a headless browser, where there is quite some concurrency in hitting Symfony (several concurrent requests to API endpoints performed by the browser almost at the same time).
From time to time, these tests fail with
request.CRITICAL: Uncaught PHP Exception ErrorException: "Notice: unserialize(): Error at offset 134 of 8192 bytes" at /var/www/vendor/symfony/http-foundation/Session/Storage/MockFileSessionStorage.php line 144Though I cannot really prove this and have no idea how to write a test case for it, my hypothesis is that several processes may try to write to the file concurrently and corrupt it.
Of course, it might also be that the problem is a read operation while another process is writing. I don't know how to tell.
The bigger solution would be to use a more involved scheme of creating the file and moving it in place, as it happens in other places in Symfony?