Navigation Menu

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

Fix memory leak in getDirectoriesFoundInManifest() #11205

Merged
merged 3 commits into from Jan 18, 2017

Conversation

triforce
Copy link
Contributor

Please issue pull request against the 3.x-dev branch only.

Piwik 2 is in LTS mode. This means we do not accept any pull request for 2.x except critical security bugs and major data loss bugs.

If you need to create a pull request for 2.x, then please also create the pull request against the 3.x-dev so we can merge both.

Happy hacking!

The 'while' loop was causing a memory leak, replaced with an 'if' to match the correct functionality.
@@ -255,13 +255,12 @@ protected static function getDirectoriesFoundInManifest()
$directory = $file;

// add this directory and each parent directory
while( ($directory = dirname($directory)) && $directory != '.' ) {
if( ($directory = dirname($directory)) && $directory != '.' ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The while was on purpose as it's supposed to add each parent directory to the array. Could you try to output the $directory variable and exit after like 10 or 15 iterations to see what's happening?

@mattab mattab modified the milestone: 3.0.2 Jan 17, 2017
@mattab
Copy link
Member

mattab commented Jan 17, 2017

this refs #11197

@Scheirle
Copy link

With:

            $count = 0;
            // add this directory and each parent directory
            while( ($directory = dirname($directory)) && $directory != '.' ) {
                $count = $count + 1;
                if ($count > 20) {
                    foreach($directories as $dic) {
                        echo $dic . "<br>";
                    }
                    break;
                }
                $directories[] = $directory;
            }

It outputs http://pastebin.com/dteAKnjm
If I count to 40 it outputs: http://pastebin.com/hy6fpqmx

Seems like dirname("/") returns / and therefore we have a endless loop.
Adding && $directory != '/' to the while loop condition works fine here.

@triforce
Copy link
Contributor Author

Done.

@mattab
Copy link
Member

mattab commented Jan 18, 2017

Thank you very much for the info & pull request fix!

@mattab mattab merged commit f5a5e39 into matomo-org:3.x-dev Jan 18, 2017
@mattab mattab added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Feb 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants