User tests: Successful: Unsuccessful:
If the session expires, it's no longer "valid" and an uncaught exception is thrown, resulting in the following:
Fatal error: Uncaught exception 'RuntimeException' with message 'The session is not started.' in /Users/tampe125/git/joomla-cms/libraries/joomla/session/handler/native.php on line 221
RuntimeException: The session is not started. in /Users/tampe125/git/joomla-cms/libraries/joomla/session/handler/native.php on line 221
Since this is the last thing performed, there no harm is done.
I think we should always save data to session, no matter what, however I'd like @mbabker to take a look at this. Since the code was already there, maybe there is an hidden reason for that?
Status | New | ⇒ | Pending |
Labels |
Added:
?
|
Yes, it is the same issue, described at the administrator login, that requires several retries until the session is properly initialized.
I confirm the Error each time I log out.
I have tested this item successfully on 8f9eecf
If this patch "fixes" the issue, IMO someone needs to dig into the underlying issue this is exposing which is basically that sessions are being saved/closed multiple times in a request cycle.
I have tested this item successfully on 8f9eecf
Solved!
If this patch "fixes" the issue, IMO someone needs to dig into the underlying issue this is exposing which is basically that sessions are being saved/closed multiple times in a request cycle.
No idea, as you may guess...
Concentrating on New Year dinner.
Status | Pending | ⇒ | Ready to Commit |
RTC - thanks
Labels |
Added:
?
|
Milestone should be 3.5.0 for this one. It is really bothering...
Agree with @infograf768 , it must be included in J3.5 Beta 2. Otherwise, it will reported multiple time.
Please, @wilsonge take note of this Fatal error
I'll echo what I said above about needing to dig deeper into the real issue. Merging this should be fine short term, maybe even just comment the code out instead of delete it, but please don't let it become another task for a rainy day.
@mbabker pull requests welcome ;)
On 27 January 2016 at 19:04, Michael Babker notifications@github.com
wrote:
I'll echo what I said above about needing to dig deeper into the real
issue. Merging this should be fine short term, maybe even just comment the
code out instead of delete it, but please don't let it become another
task for
https://github.com/joomla/joomla-cms/blob/a0b215250bb140092ec13d529328ceb6812c275d/plugins/user/joomla/joomla.php#L93 a
rainy day
fc9ec7a#diff-8b42e55d5e424b7f172d227f1a012e1dR72
.—
Reply to this email directly or view it on GitHub
#8808 (comment).
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
Accept my other ones first and maybe we'll talk
damned gloabl warming
I already made a decision I'm not merging this for beta 2 for exactly the reasons michael mentioned. I will mention this and the utf8mb4 as "known issues" however
Shall we close it then?
Status | Ready to Commit | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-01-27 19:38:54 |
Closed_By | ⇒ | roland-d |
Status | Closed | ⇒ | New |
Closed_Date | 2016-01-27 19:38:54 | ⇒ | |
Closed_By | roland-d | ⇒ | |
Labels |
Removed:
?
|
Status | New | ⇒ | Pending |
Status | Pending | ⇒ | Closed |
OK, I'll give you this for free. Some ability to read C code is required.
Using PHP 5.3.10 (Joomla's minimum), PHP's session_write_close() function is a wrapper around an internal C function in the session extension, php_session_flush(). The latter function will only write if the session is active (that status is only available to userland code in PHP 5.4 or later). In lieu of being able to check PHP_SESSION_ACTIVE === session_status()
(PHP 5.4 code), the two options are to either remove the check in full or instead of throwing an Exception if the session is not started (according to the Joomla API, not something internal to PHP) to just early return and not trigger the session_write_close()
function. FWIW these two C functions are mostly unchanged in PHP 7.0.2 also, so there's no change in behavior across PHP branches to account for.
Thank you @mbabker for the feedback.
@tampe125 Can you update the PR to wrap the session_write_close in the started check? I believe it should be like this:
if (!$this->isStarted())
{
session_write_close();
}
That worked for me. The docblock should be updated as well since we no longer throw an exception.
Let me know if you have time to do it, otherwise I will create a new PR. Thanks.
Don't do it just around session_write_close()
. Either wrap the full method (because there are other side effects caused by calling it) or don't do a status check at all because the API to get the active status is not exposed to userland PHP code in PHP 5.3 (when 5.4 is the minimum do the check I noted).
Or, if you really want to do a check if the session is active (as PHP defines it, because that's what the underlying code is reliant on, not Joomla's state tracking for sessions), do a check similar to what's in the start method (checks status with PHP 5.4 API if able and a fallback mechanism for 5.3).
Status | New | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-01-28 16:02:26 |
Closed_By | ⇒ | roland-d |
IIRC it's supposed to prevent deep logic errors. At some point the session has to be started to be able to save it, and it shouldn't be trying to save the data multiple times in a single request under normal operation. In the context of the CMS it's probably fine to drop it though.