Out of memory (possibly memory leak?) on long running scripts.

[shaqman]

class OpController extends Controller {
    public function actionBug() {
        while (true) {
            \Yii::info('Test');
        }
    }
}

Consider the code above, it would cause a out of memory exception on https://github.com/bedezign/yii2-audit/blob/713d9f00f073f76202f2c25fab165a3adaa97fbb/src/LogTarget.php#L72.

I'm not sure whether it happens because it doesn't flush data directly or due to an exception when saving data to database, but adjusting the code to be as following fixed it for me. Of course logs would then be lost, but having multiple log targets solves the issue (eg: FileTarget).

public function export() {
    ...
   // remove  $this->messages = [];
    }

    /**
     * @param array $messages
     * @param bool  $final
     */
    public function collect($messages, $final) {
        $this->messages = array_merge($this->messages, static::filterMessages($messages, $this->getLevels(), $this->categories, $this->except));
        $count = count($this->messages);
        if ($count > 0 && ($final || $this->exportInterval > 0 && $count >= $this->exportInterval)) {
            // set exportInterval to 0 to avoid triggering export again while exporting
            $oldExportInterval = $this->exportInterval;
            $this->exportInterval = 0;
            $this->export();
            $this->exportInterval = $oldExportInterval;
        }
        $this->messages = [];
    }

Any better ideas, or should I just make a PR for this?

[Blizzke]

The reason we only export on final is so that all writing to the database is done after the request was completely finished and the output was sent. It takes time to save everything and that would be noticeable in the response times.

It is possible to add intermediate exporting to the code, but only when this behavior is disabled by default to guarantee backwards compatibility. Then people would have to explicitly enable it to use it and know the consequences.

[shaqman]

Actually, isn't that already configurable from the parent target class? We could keep the code as I propose above, but also set exportInterval default value as 0.

That way, doesn't it means that by default, it still works like you said, but for others it is configurable. This should be better than just disabling the log target, which is what I did as a workaround for the moment.

Edit: I must need my doze of coffee. 0 does not mean flushing directly. But the idea still remains and I do agree with your point. Default should be as it is, but make it configurable so that for certain cases we could adjust it.

[Blizzke]

Your code is fine, but we should probably add a property indicating whether it is allowed to export during the request with the default value set to false. That should suffice to warrant the same behavior as before, while giving people that log a lot the possibility to configure a flushing threshold. Good idea.

[lluisclava]

Hello,

Is there any solution about this? I have the same problem in console app.

I tried this:

Config file: 'modules'=>[ 'audit' => [ 'class'=>'bedezign\yii2\audit\Audit', 'ignoreActions' => ['*'], ] ],

The action of the controller I called: for($i=0;$i<=1000;$i++){ $model=new \backend\models\SomeModel; unset($model); echo "memory: ".(memory_get_usage(true)/1024/1024)." MiB".PHP_EOL; }

You can see all the time memory usage is growing.

I just wont in the console use the module to cleanup database and send e-mail messages, not to audit anything.....

Thanks in advance!

[lluisclava]

Hello,

Is there any solution about this? I have the same problem in console app.

I tried this:

Config file: 'modules'=>[ 'audit' => [ 'class'=>'bedezign\yii2\audit\Audit', 'ignoreActions' => ['*'], ] ],

The action of the controller I called: for($i=0;$i<=1000;$i++){ $model=new \backend\models\SomeModel; unset($model); echo "memory: ".(memory_get_usage(true)/1024/1024)." MiB".PHP_EOL; }

You can see all the time memory usage is growing.

I just wont in the console use the module to cleanup database and send e-mail messages, not to audit anything.....

Thanks in advance!

Finally, I decided to add a property to disable auditing: `public function init() { parent::init(); $app = Yii::$app;

    // check if the module has been installed (prevents errors while installing)
    try {
        $this->getDb()->getTableSchema(AuditEntry::tableName());
    } catch (\Exception $e) {
        return;
    }

    **if(!$this->noAudit){
        // Before action triggers a new audit entry
        $app->on(Application::EVENT_BEFORE_ACTION, [$this, 'onBeforeAction']);
        // After request finalizes the audit entry.
        $app->on(Application::EVENT_AFTER_REQUEST, [$this, 'onAfterRequest']);

        $this->activateLogTarget();
    }**

    // Boot all active panels
    $this->normalizePanelConfiguration();
    $this->panels = $this->loadPanels(array_keys($this->panels));
}`

This solves my problem.

[handcode]

@Blizzke any plan to release version with added 'exportAtIntervals' from https://github.com/bedezign/yii2-audit/commit/9d44f74c90ddd270285ef6accb6312b482d5a7ee

With current 'only export on final' we have to disable (unset) Yii::$app->getLog()->targets['audit']in various scripts as collecting all messages on the stack is completly unuseable for scripts that do "a lot".

A good example for such a script is yii audit/cleanup --interactive=0 --entrySolo with "many" entries.

In my tests, the memory footprint increased by approximately 10 MB per 1000 entries. Then calculate what it requires to clean up an audit_entry table with 3 million entries ....

Cc: @schmunk42

[handcode]

@Blizzke sorry, missed https://github.com/bedezign/yii2-audit/commit/6abe0f5b8e8e1e44e2bd16261c8e5bc948bd67fb