8000 Log file not closed causing "failed to open stream: Too many open files" using phpunit-bridge · Issue #17268 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content
8000

Log file not closed causing "failed to open stream: Too many open files" using phpunit-bridge #17268

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

Jump to bottom
Closed
Congelli501 opened this issue Jan 5, 2016 · 3 comments
Closed

Log file not closed causing "failed to open stream: Too many open files" using phpunit-bridge #17268

Congelli501 opened this issue Jan 5, 2016 · 3 comments
Labels
PhpUnitBridge

Comments

@Congelli501
Copy link

When running a large phpunit test with phpunit-bridge installed, the log files (app/logs/test.log) is not closed and the php process ends up failing with the failed to open stream: Too many open files error.

lsof | grep '^php' | grep 'test.log$' | wc -l returns 700+ results.

This bug only appear when symfony/phpunit-bridge is installed.
Removing it from my composer.json and running the tests again makes the tests run well, with ~20 logs files open.

Could it be caused by "It disables PHP's garbage collector for tests to avoid segmentation faults;" (source: http://symfony.com/blog/new-in-symfony-2-7-phpunit-bridge) ?

Symfony version: 2.7.8
PHP version: 5.6.11 (from Ubuntu 15.10)
phpunit version: 5.1.3 (installed via composer)

This seems to be related to this bug: #11566
@nicolas-grekas
Copy link
Member

Thanks for the detailed report and the analyze. Did you try to re-enable the gc?

@stof
Copy link
Member
stof commented Jan 5, 2016

This might totally be related, as the GC is needed to clean circular object graphs.

@nicolas-grekas I don't think that the bridge should force disabling the gc for anyone using it. This should be done only in the Symfony testsuite (or even not at all if we don't have segfaults because of it anymore, as this might have been fixed in PHP)

@Congelli501
Copy link
Author

@nicolas-grekas I re-enabled gc, and this is definitely the source of the unclosed files.

I added these lines at the beginning of the first run test:

var_dump(gc_enabled());
gc_enable();
var_dump(gc_enabled());

With symfony/phpunit-bridge enabled:

bool(false) # gc is disabled at start
bool(true)
< Tests run as expected, even if symfony/phpunit-bridge is installed >

Without symfony/phpunit-bridge:

bool(true) # gc is enabled at start
bool(true)
< Tests run as expected >

@stof I agree that gc should be enabled for unit tests. I expect the tests to be run in an environment as close as possible to production. If the code makes PHP crash, the tests should show it rather than hide it.

I have never experienced any segmentation fault during unit tests, with recent Ubuntu & Debian releases (14.04+ / 7+). This seems to be related to this PHP issue, closed in 2012: https://bugs.php.net/bug.php?id=60825

@nicolas-grekas nicolas-grekas mentioned this issue Jan 6, 2016
Merged
nicolas-grekas added a commit that referenced this issue Jan 6, 2016
@nicolas-grekas
bug #17275 [PhpUnitBridge] Re-enable the garbage collector (nicolas-g…
21427de 
…rekas)

This PR was merged into the 2.7 branch.

Discussion
----------

[PhpUnitBridge] Re-enable the garbage collector

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #17268, #11566
| License       | MIT
| Doc PR        | -

The bug is fixed in PHP and disabling the gc causes issues elsewhere.

Commits
-------

478710f [PhpUnitBridge] Re-enable the garbage collector
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
PhpUnitBridge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@nicolas-grekas @stof @Congelli501 @xabbuh
0