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

Memory leaks with Opcache #50

Closed
ghost opened this Issue Apr 22, 2014 · 11 comments

Comments

Projects
None yet
4 participants
@ghost

ghost commented Apr 22, 2014

In PHP 5.5 with the builtin Opcache enabled for the CLI using the CronExpression causes memory leaks.

opcache.enable_cli = 1

Execute the following script on the command line:

use Cron\CronExpression;

$expression = CronExpression::factory('25 0 1 * *');

while (true) {
    $expression->isDue();

    usleep(100000);
}

The used memory (RSS) will increase rapidly and PHP will eventually throw a fatal error.

Please note that when changing the expression to * * * * * this issue does not occur.

It may very well be an issue with the Opcache, however I haven't been able to pinpoint exactly what is happening here.

Tested on CentOS 6.4 and Mac OS X 10.9.2 with PHP 5.5.10 and PHP 5.5.11

@mtdowling

This comment has been minimized.

Show comment
Hide comment
@mtdowling

mtdowling Apr 22, 2014

Owner

Do you still observe memory leak when the Opcache is disabled?

Owner

mtdowling commented Apr 22, 2014

Do you still observe memory leak when the Opcache is disabled?

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Apr 22, 2014

No. When the Opcache is disabled this problem does not occur.

ghost commented Apr 22, 2014

No. When the Opcache is disabled this problem does not occur.

@mtdowling

This comment has been minimized.

Show comment
Hide comment
@mtdowling

mtdowling Apr 22, 2014

Owner

Hm... I'm not sure what I can do as a library author here.

What happens when you add gc_collect_cycles() to your loop?

Owner

mtdowling commented Apr 22, 2014

Hm... I'm not sure what I can do as a library author here.

What happens when you add gc_collect_cycles() to your loop?

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Apr 22, 2014

gc_collect_cycles() did not make a difference.

I understand this may not be related to CronExpression but perhaps somebody else might understand what is causing this problem. I opened up a ticket to share this information here and hopefully save other people some debugging hours.

My current solution is to disable to Opcache CLI, however it would be nice to understand why this is happening.

ghost commented Apr 22, 2014

gc_collect_cycles() did not make a difference.

I understand this may not be related to CronExpression but perhaps somebody else might understand what is causing this problem. I opened up a ticket to share this information here and hopefully save other people some debugging hours.

My current solution is to disable to Opcache CLI, however it would be nice to understand why this is happening.

@viktoras25

This comment has been minimized.

Show comment
Hide comment
@viktoras25

viktoras25 Apr 29, 2014

Contributor

So as I noted in PR #51, the problem was in continue 2 line. I haven't find any smarter way other than using goto, which solved the problem for me. Of course this is not the way to go for the library, but if you need this problem solved anyway you can refer here: https://github.com/charnad/cron-expression/blob/issue-50/src/Cron/CronExpression.php#L310.
Or wait until PHP fixes it.

Contributor

viktoras25 commented Apr 29, 2014

So as I noted in PR #51, the problem was in continue 2 line. I haven't find any smarter way other than using goto, which solved the problem for me. Of course this is not the way to go for the library, but if you need this problem solved anyway you can refer here: https://github.com/charnad/cron-expression/blob/issue-50/src/Cron/CronExpression.php#L310.
Or wait until PHP fixes it.

@recipe

This comment has been minimized.

Show comment
Hide comment
@recipe

recipe Sep 19, 2014

Contributor

I think, isDue() implementation is too costly. It calculates next run date itself and compares it with the specified date. It would be much quicker if isDue() implementation just called isSatisfiedBy() method for each chunk of the expression (each Field).

Contributor

recipe commented Sep 19, 2014

I think, isDue() implementation is too costly. It calculates next run date itself and compares it with the specified date. It would be much quicker if isDue() implementation just called isSatisfiedBy() method for each chunk of the expression (each Field).

@mtdowling

This comment has been minimized.

Show comment
Hide comment
@mtdowling

mtdowling Sep 19, 2014

Owner

@recipe I think that could work

Owner

mtdowling commented Sep 19, 2014

@recipe I think that could work

@fruitl00p

This comment has been minimized.

Show comment
Hide comment
@fruitl00p

fruitl00p Dec 19, 2014

Contributor

FYI: the problem still persists with PHP5.6.3 and the goto-hack works as intended!

Contributor

fruitl00p commented Dec 19, 2014

FYI: the problem still persists with PHP5.6.3 and the goto-hack works as intended!

@fruitl00p

This comment has been minimized.

Show comment
Hide comment
@fruitl00p
Contributor

fruitl00p commented Dec 22, 2014

@ghost

This comment has been minimized.

Show comment
Hide comment
@mtdowling

This comment has been minimized.

Show comment
Hide comment
@mtdowling

mtdowling Oct 11, 2015

Owner

With this fixed in PHP, I think this can be closed.

Owner

mtdowling commented Oct 11, 2015

With this fixed in PHP, I think this can be closed.

@mtdowling mtdowling closed this Oct 11, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment