Page MenuHomePhabricator

Tests leak memory under PHP 7.4 and 8
Closed, ResolvedPublic

Description

When running under PHP 8 (revision a8e8c40ad4), tests eventually run out of memory with a message like

mmap() failed: [12] Cannot allocate memory

mmap() failed: [12] Cannot allocate memory

Fatal error: Out of memory (allocated 2204106752) (tried to allocate 20480 bytes) in /vagrant/mediawiki/includes/ServiceWiring.php on line 780

Memory usage profiling of several test cases via php-memprof suggests that what's not getting released are service wirings loaded in ServiceContainer::loadWiringFiles():

image.png (258×1 px, 37 KB)

Event Timeline

@MaxSem I wonder if this is somehow related to https://bugs.php.net/bug.php?id=76982. Granted, that one seems to have been present on PHP 7.x as well, but perhaps PHP 8 exacerbated the issue.

https://github.com/php/php-src/commit/0f2cdbf214efd98b4bdaf5ca41728faf00e7c037 looks to be a likely culprit (introduced as a fix for PHP bug 78903). Specifically the monotonically increasing rtd_counter appears to mean that each time a file containing a closure is included, a new entry will be generated in the function table for the closure. After some quick & dirty testing locally, removing the counter seems to stabilize memory usage.

I tried to debug this a bit today, and would like to post the results here in case it may be useful to somebody with better PHP internals knowledge than me (and that's a very low bar!)

Running the reproduction scripts from bug 76982 with PHP 8 master on macOS yielded me the following in leaks:

STACK OF 1 INSTANCE OF 'ROOT LEAK: malloc<5242880>':
28  libdyld.dylib                      0x7fff73a8e7fd start + 1
27  php                                   0x10fcf7eb0 main + 1872  php_cli.c:1351
26  php                                   0x10fcf8e02 do_cli + 3442  php_cli.c:956
25  php                                   0x10fb301cb php_execute_script + 955  main.c:2584
24  php                                   0x10fbe0e63 zend_execute_scripts + 643  zend.c:1663
23  php                                   0x10fc5761b zend_execute + 283  zend_vm_execute.h:56144
22  php                                   0x10fc573e4 execute_ex + 100  zend_vm_execute.h:51849
21  php                                   0x10fc9f67a ZEND_INCLUDE_OR_EVAL_SPEC_CONST_HANDLER + 58  zend_vm_execute.h:3985
20  php                                   0x10fcf11a6 zend_include_or_eval + 982  zend_execute.c:4159
19  php                                   0x10fb77ebe compile_filename + 190  zend_language_scanner.l:671
18  php                                   0x10f938b9c phar_compile_file + 1148  phar.c:3299
17  php                                   0x10fb77bf3 compile_file + 163  zend_language_scanner.l:650
16  php                                   0x10fb77d41 zend_compile + 289  zend_language_scanner.l:614
15  php                                   0x10fbb6356 zend_compile_top_stmt + 102  zend_compile.c:8705
14  php                                   0x10fbb6404 zend_compile_top_stmt + 276  zend_compile.c:0
13  php                                   0x10fbae74f zend_compile_stmt + 815  zend_compile.c:8755
12  php                                   0x10fbad412 zend_compile_return + 210  zend_compile.c:0
11  php                                   0x10fba5f8a zend_compile_expr + 1642  zend_compile.c:8951
10  php                                   0x10fbb918d zend_compile_array + 573  zend_compile.c:8211
9   php                                   0x10fba5ff8 zend_compile_expr + 1752  zend_compile.c:8970
8   php                                   0x10fbb3538 zend_compile_func_decl + 648  zend_compile.c:6294
7   php                                   0x10fbb3a9e zend_begin_func_decl + 606  zend_compile.c:6229
6   php                                   0x10fbb3024 zend_hash_add_ptr + 52  zend_hash.h:593
5   php                                   0x10fbf8d4a zend_hash_add + 42  zend_hash.c:874
4   php                                   0x10fbf9260 _zend_hash_add_or_update_i + 1152  zend_hash.c:772
3   php                                   0x10fc00869 zend_hash_do_resize + 345  zend_hash.c:1164
2   php                                   0x10fb9cae5 __zend_malloc + 21  zend_alloc.c:2976
1   libsystem_malloc.dylib             0x7fff73c43fe5 malloc + 21
0   libsystem_malloc.dylib             0x7fff73c4408e malloc_zone_malloc + 140

This does seem to indicate that one of the leak sources is PHP's function table; due to bug 78903, each time a file with closure declaration(s) is included, PHP will add a new entry to the function table for the closures. However, this doesn't seem to be the only leak source, as jerry-rigging this will make the report go away from leaks but the test script still OOMs eventually (although it appears to do so slightly slower).

Unfortunately I couldn't figure out how to approach fixing the problem, but perhaps this information may be useful for someone who can.

MaxSem renamed this task from Tests leak memory under PHP 8 to Tests leak memory under PHP 7.4 and 8.Apr 22 2020, 8:35 PM
MaxSem added a project: PHP 7.4 support.

Is there much we can do about this one? Other than waitng for an upstream fix, and a possible backport/similar?

Is there much we can do about this one? Other than waitng for an upstream fix, and a possible backport/similar?

I've tried adjusting MediaWikiIntegrationTestCase::installMockMwServices to clone the default service container, as opposed to populating a new instance from wiring files, when not given any config overrides. On PHP 8, this dropped memory usage to 894.25 MB from 4+ GB when running the MediaWiki core PHPUnit tests. Unfortunately, this causes a few test failures with MediaWiki\Storage\NameTableAccessException: Failed to access name from slot_roles using id = 2, which will need some investigation.

Reedy added a subscriber: daniel.

Tagging Platform Engineering as slot_roles is Platform Team Initiatives (MCR) related, so @daniel wrote it and might have a bit of insight

Could be worth uploading your WIP patch to gerrit and tagging it here for reference :)

Change 592471 had a related patch set uploaded (by TK-999; owner: TK-999):
[mediawiki/core@master] [WIP] Workaround PHP 7.4>= memory leak in tests

https://gerrit.wikimedia.org/r/592471

Is there much we can do about this one? Other than waitng for an upstream fix, and a possible backport/similar?

I've tried adjusting MediaWikiIntegrationTestCase::installMockMwServices to clone the default service container, as opposed to populating a new instance from wiring files, when not given any config overrides.

That seems risky. I'd rather find a way to cache the wiring array itself, so we don't re-create all these closures for every test.

eprodromou triaged this task as High priority.

Is there much we can do about this one? Other than waitng for an upstream fix, and a possible backport/similar?

I've tried adjusting MediaWikiIntegrationTestCase::installMockMwServices to clone the default service container, as opposed to populating a new instance from wiring files, when not given any config overrides.

That seems risky. I'd rather find a way to cache the wiring array itself, so we don't re-create all these closures for every test.

Thanks! I have revised the patch accordingly. And now the tests pass :)

Change 592471 merged by jenkins-bot:
[mediawiki/core@master] [WIP] Workaround PHP 7.4>= memory leak in tests

https://gerrit.wikimedia.org/r/592471