Skip to content

Memory Leak #584

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.

Sign up for GitHub

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

Open
ihar-aleksiaichuk wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Memory Leak #584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2abd623
fixed:
ihar-aleksiaichuk Feb 12, 2019
a034c02
changed version
ihar-aleksiaichuk Feb 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions common/log/class.SingleFileAppender.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,17 @@ protected function initFile()
{
if ($this->maxFileSize > 0 && file_exists($this->filename) && filesize($this->filename) >= $this->maxFileSize) {
// need to reduce the file size
$file = file($this->filename);
$file = array_splice($file, ceil(count($file) * $this->reduceRatio));
$reduceLen= ($this->maxFileSize< (int)ini_get('memory_limit')*1024?$this->maxFileSize:(int)ini_get('memory_limit')*1024)*$this->reduceRatio;

@martijn-tao martijn-tao Feb 12, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For readability it might be better to change this from the shorthand if else to the full controll structure:

if ($this->maxFileSize < (int)ini_get('memory_limit') * 1024) {
    $reduceLength = ($this->maxFileSize) * $this->reduceRatio;
} else {
    $reduceLength = ((int)ini_get('memory_limit') * 1024) * $this->reduceRatio;
}

@alexeyvy alexeyvy Feb 12, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A variable for (int)ini_get('memory_limit') * 1024 wouldn't also be out of place here

$size = filesize($this->filename)-$reduceLen;

@martijn-tao martijn-tao Feb 12, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please surround mathematical operators (+, -, *, /, %) with spaces

$log =file_get_contents($this->filename, false, null, $size, $reduceLen );

@martijn-tao martijn-tao Feb 12, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Try to be consistent with spacing. For example always surround = with spaces (eg. $log = file_get_contents(...)

@martijn-tao martijn-tao Feb 12, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also please omit the whitespace after the last argument (in this case $reduceLen)


$log= explode(PHP_EOL, $log);
//delete first element (can be broken)
array_shift($log);

$this->filehandle = @fopen($this->filename, 'w');
foreach ($file as $line) {
@fwrite($this->filehandle, $line);
foreach ($log as $line) {
@fwrite($this->filehandle, $line.PHP_EOL);

@martijn-tao martijn-tao Feb 12, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As per the PSR-2 standard, please surround the concatenation sign (.) with spaces.

$line . PHP_EOL

}
} else {
$this->filehandle = @fopen($this->filename, 'a');
Expand Down
2 changes: 1 addition & 1 deletion manifest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
'label' => 'Generis Core',
'description' => 'Core extension, provide the low level framework and an API to manage ontologies',
'license' => 'GPL-2.0',
'version' => '8.2.1',
'version' => '8.2.2',
'author' => 'Open Assessment Technologies, CRP Henri Tudor',
'requires' => array(),
'models' => array(
Expand Down
2 changes: 1 addition & 1 deletion scripts/update/Updater.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,6 @@ public function update($initialVersion)
$this->setVersion('8.0.0');
}

$this->skip('8.0.0', '8.2.1');
$this->skip('8.0.0', '8.2.2');
}
}