Skip to content
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

$app->registeredurlparams is not defined #3220

Closed
wants to merge 1 commit into from
Closed

Conversation

@Hackwar
Copy link
Member

@Hackwar Hackwar commented Mar 2, 2014

Moving the rest of the function inside the if loop makes sure $app->registeredurlparams is only referred to if actually exists.

This was causing memory leak on 3.2.1 Joomla install over PHP 5.5, performance has increased after the fix.

This is a replacement for #2706 with a better fix, since there was no response from @FriendlyHacker in 2 months. Thank you for your work.

@wilsonge
Copy link
Contributor

@wilsonge wilsonge commented Mar 3, 2014

I know it's not really your submission but is there any easy way of replicating this Hannes? (obviously from a code perspective it's correct - but just to do the official testing crap)

@FriendlyHacker
Copy link

@FriendlyHacker FriendlyHacker commented Mar 7, 2014

I can confirm this fixes the issue, and has not broken anything else so far.

@FriendlyHacker
Copy link

@FriendlyHacker FriendlyHacker commented Mar 7, 2014

This is how you replicate it:

1 - Enable warnings to be written to PHP error log.
2 - Visit your site a few times
3 - Open error log
4 - Find PHP Warning: Creating default object from empty value in libraries/joomla/cache/cache.php on line 702
5 - Apply fix
6 - Watch Joomla performance double

For extra effect, clear your error log file. This warning message gets written every time someone visits and you might have a huge log file consuming all your server's RAM because of it.

@Bakual Bakual closed this in e36643c May 25, 2014
@Hackwar Hackwar deleted the Hackwar:patch-8 branch Jan 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants