worker: add test on lazy session save path#2203
Conversation
|
I've tried this out, but it turns out that none of the ini_set directives you had mentioned are even getting hit. |
|
@henderkes not sure what you meant, I've tried with a real sf 8 app and the bug came from the |
|
I've tried debugging a demo application with |
|
I used the following config, the path comes from the issue session:
handler_id: null
cookie_secure: false
save_path: '%kernel.project_dir%/var/sessions/%kernel.environment%' |
save_path: '%kernel.project_dir%/var/sessions/%kernel.environment%'That explains it, there's conditional logic prior if the path is different. However, since it was never hitting these code paths for me and I still ran into issues, that cannot be the whole store. There's got to be a bug somewhere unrelated to the ini settings as well. |
|
I'm in favor of reverting the INI stuff entirely and documenting explicitly about what is reset and what isn't. INI settings are not request-bound, so there is no reason to reset them. If Symfony does stuff like this, it's entirely sure that many existing apps also do it. |
|
@henderkes defaut in symfony is |
|
I think a potential failure could also come from not being able to reset an ini setting at the end of a request (for various reasons). So yeah, the safe bet would be to just not reset them and document 8000 the behavior. We also don't reset global variables in general, so probably makes sense to count ini settings to global variables. |
|
I've changed my mind on this, We'd indeed have to check too many conditions to be able to safely reset ini settings. @xavierleune would you mind reverting the ini changes specifically, or should we simply remove the whole commit and amend the non-conflicting session changes with tests later? @dunglas we should really look into doing a patch release in the very near future. Having both build (albeit not to our fault) and functionality broken is not great. |
|
I agree ;) |
|
thanks everyone for your help and sorry about the bugs, closing this PR |
This small reproducer should reproduce the issue #2185
This bug has probably be introduced with the PR #2139: we now reset ini settings between requests and symfony sets the save_path only on the first use of the session object.
We have a few possible ways to fix it here:
ini_setare always calledI went through all the symfony code base, we have the following
ini_set:$unserializeCallbackHandler = ini_set('unserialize_callback_func', __CLASS__.'::handleUnserializeCallback'); /* do stuff */ ini_set('unserialize_callback_func', $unserializeCallbackHandler);)enable: not normally used in worker mode@dunglas @AlliBalliBaba @henderkes WDYT ?