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():


Event Timeline

MaxSem created this task.Mar 18 2020, 3:19 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 18 2020, 3:19 PM
TK-999 added a subscriber: TK-999.Apr 3 2020, 11:27 PM

@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.

TK-999 added a comment.Apr 4 2020, 1:39 PM

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.
Reedy moved this task from Backlog to MediaWiki core on the PHP 7.4 support board.Apr 23 2020, 2:47 AM
Reedy added a subscriber: Reedy.Apr 24 2020, 3:52 PM

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 assigned this task to daniel.Apr 28 2020, 8:38 PM
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

Naike closed this task as Resolved.May 22 2020, 6:55 AM