User tests: Successful: Unsuccessful:
This is a redo of PR #8808 based on the feedback by @mbabker
A signal for @wilsonge :)
Status | New | ⇒ | Pending |
Labels |
Added:
?
|
I have tested this item successfully on bbb6f33
Tested on php 5.5.26 the fatal error has now gone
But I still have to login twice and am not returned to the page I was on when the session timed out
@roland-d I have tested this successfully as described above in test instructions, BUT:
When updating from 3.4.8 using a patched Beta 2 update container wia Joomla Update (custom URL with patched XML), the update ends with following display:
And the error log file of PHP contains following error:
[28-Jan-2016 17:43:28 Europe/Berlin] PHP Fatal error: Uncaught exception 'RuntimeException' with message 'The session is not started.' in /homepages/xx/yy/htdocs/test1/libraries/joomla/session/handler/native.php:221
Stack trace:
#0 [internal function]: JSessionHandlerNative->save()
#1 {main}
thrown in /homepages/xx/yy/htdocs/test1/libraries/joomla/session/handler/native.php on line 221
Please advice if I shall set my test result to "success" because this is another issue or cannot be avoided when upgrading from a pre-3.5, or to "failed" because you wanna fix that here, too.
P.S.: My PHP version is 5.6.15
@richard67 That error comes from the old native.php file. Line 221 only has a curly brace on it once this patch is applied.
A timeout during update, sounds odd though. Perhaps that is another issue. Your test is good I would say, since we need to test with the new native.php file.
I have tested this item successfully on bbb6f33
Tested with success as described in test instructions, PHP version used for the test was 5.6.15.
Unfortunately I have no PHP 5.3 anymore so I cannot test for this, but from my point of view it would make sense to have a good test also for PHP 5.3.
All that points to is what I was saying in the last pull request; something is trying to start/shutdown the session more than the expected once per request cycle. That error specifically isn't related to this pull request, but it needs to be dug into. If someone who is experiencing these issues can open a new bug report and include a stack trace (the debug_backtrace()
function will do the trick) showing the paths that are causing JSession::_start()
and JSession::close()
to be called would be helpful. From a cursory glance, there may be a path where JSession::restart()
, JSession::destroy()
, or JSession::fork()
get called that may be able to cause a glitch but it's just hypothetical at the moment.
Hello Michael,
When I debugged this, the stack trace wouldn't show anything, it just shows this:
When I step through it I went from
$app = JFactory::getApplication('administrator');
in index.php to
if (!$this->_validate())
in session.php. The validation comes back as false because more than 60 seconds has passed in my case. This then calls:
$this->destroy();
in session.php which ends up at the save() method in native.php because that is registered as shutdown function.
Since the session hasn't started (it was destroyed earlier) it now fails the check:
if (!$this->isStarted())
in native.php, throwing out the RuntimeException.
That is what I dug up.
@mbabker See also my error log above with the problem on update from 3.4.8 and the new native.php: A stack trace was printed out, but is shows only the 1 frame for JSessionHandlerNative->save(). And I am not familiar with debug_backtrace(), so I have the issue but am not sure if I can be the one who digs into this deeper. But if you want I can at least open an issue for that in the issue tracker.
When something is triggered as a shutdown function it won't have a stack trace. So if the first occurrence is at that point then the shutdown handler shouldn't be the issue unless there's a bug in the shutdown workflow in general. This patch is fine specifically for the error about writing the session data when the session isn't started (according to Joomla's state). But there still seems to be some deep rooted logic error if the scenario @richard67 tested results in trying to start the session multiple times (meaning there's a disconnect between Joomla's internal state tracking and PHP's own active status).
I have tested this item successfully on bbb6f33
Status | Pending | ⇒ | Ready to Commit |
Labels |
Added:
?
|
Milestone |
Added: |
Fatal error is gone here but I still have to log twice here.
Warning
Your session has expired. Please log in again.
@brianteeman This PR is only for fixing the fatal error.
Status | Ready to Commit | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-01-29 09:23:15 |
Closed_By | ⇒ | Kubik-Rubik | |
Labels |
Added:
?
|
Labels |
Removed:
?
|
This PR supposed to be merged but just tested and verified issue without patching.
Patch not available to apply. Still new at this so will await TeamJUGL verification.
It has been merged so it is no longer present in the list of patches displayed by com_patchtester
Correct but the issue remained on Joomla downloaded from Github this morning
Yes it will be - but if you look it says merged
On 30 January 2016 at 10:48, Gary Barclay notifications@github.com wrote:
Correct but the issue remained on Joomla download from Github this morning
—
Reply to this email directly or view it on GitHub
#9011 (comment).
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
Doesn't that mean the patch is applied to the staging version?
yes
On 30 January 2016 at 10:50, Gary Barclay notifications@github.com wrote:
Doesn't that mean the patch is applied to the staging version?
—
Reply to this email directly or view it on GitHub
#9011 (comment).
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
Two comments:
1) Remove the
@throws
line in doc block2) Change the
// If running PHP 5.4, try to use the native API
comment to explain what is actually being checked (you're verifying the session is actually active)