Follow-up to #2921862: Segfault on PHP5.5 and PostgreSQL. We added gc_disable() between BrowserTestBase::registerSessions() and BrowserTestBase::setUp() because it improved stability with tests. We need discover the reasons, similar to #2842393: Discover why gc_disable() improves update.php stability.
Original post
Problem/Motivation
Sometimes, collect the dependencies leads to a random fail with PHP 5.5 and PHP 5.6 on Drupal CI (not with PHP 7.0, 7.1).
Example with only PHP 5.6 https://www.drupal.org/pift-ci-job/732367:
testMultiValuedWidget
fail: [Completion check] Line 134 of core/modules/file/src/Tests/FileFieldWidgetTest.php:
The test did not complete due to a fatal error.
Example with only PHP 5.5/SQLite https://www.drupal.org/pift-ci-job/805228:
1) Drupal\Tests\hal\Functional\EntityResource\EntityTest\EntityTestHalJsonInternalPropertyNormalizerTest::testDelete
PHPUnit_Framework_Exception: Segmentation fault (core dumped)
Example with only PHP 5.5/Postgresql https://www.drupal.org/pift-ci-job/805466:
1) Drupal\Tests\rest\Functional\EntityResource\EntityTest\EntityTestJsonInternalPropertyNormalizerTest::testDelete
PHPUnit_Framework_Exception: Segmentation fault (core dumped)
The Story:
(Correct, please, if something is missing)
⏰ One day, we reverted the patch #2849674-57: Complex job in ViewExecutable::unserialize() causes data corruption, because it led to a frequent random fail in FileFieldWidgetTest.
🛠 It was unexpected, and we opened a separate issue for clarifying the reasons #2898721: Random segfault currently in FileFieldWidgetTest::testMultiValuedWidget().
⚔ We also started experiments in the #2879048-57: Ignore: patch testing issue for #2919863 (see from #57) for hide excessive noise during a blind battle with Drupal CI (the fail did not work locally).
📌 #2849674-66: @tacituseu noticed that a new user_batch_action_test_action module from patch, it is enough to trigger fail.
📌 #2898721-28: @xjm pointed to a possibly related PHP bugs: https://bugs.php.net/bug.php?id=72286 and https://bugs.php.net/bug.php?id=71958.
📌 2879048-57: @vaplas checked the work fail-patch 200 times for PHP 5.5 and 7.1, but not fail.
📌 2879048-67: @tacituseu checked the work fail-patch for all versions of PHP on DrupalCI, but the problem manifested itself only on PHP 5.6.
👍 2879048-102: @Lendude found the minimum code for reproducing the fail:
--- /dev/null
+++ b/core/modules/views/tests/modules/user_batch_action_test/user_batch_action_test.info.yml
@@ -0,0 +1,9 @@
+name: 'User batch action test'
+type: module
+description: 'Support module for user batch action testing.'
+package: Testing
+version: VERSION
+core: 8.x
+dependencies:
+ - views
+ - user
🥊 2879048-102 - 2879048-126: after that, we tested a lot of combinations with add/remove/reorder of dependencies. The error disappeared and appeared in an unpredictable manner.
💡 2879048-127: @tacituseu great explanation:
The fails are in circular reference garbage collector, my guess would be it either:
1. changes order of objects and the algorithm has easier time collecting them
2, increases reference count so the algorithm doesn't even try to collect itAlso follow up on php versions used for testbots (drupalci_environments): they are based on tarballs directly from https://secure.php.net, except for those suffixed with 'x-dev' which are from https://github.com/php/php-src/, so no Ubuntu/Debian specific patches in them.
💎 2879048-129: @tacituseu proved it via keeping references to byproducts of core/lib/Drupal/Core/Extension/ModuleHandler.php::buildModuleDependencies(), this fixed the fail (see also #2842393-23: Discover why gc_disable() improves update.php stability).
🔧 2879048-137: @tacituseu made patch with save graph of dependencies for compare fail and pass situations. See also 2879048-140.
⏳ pause
😢 2879048-156 Unfortunately, easy reproducing fail stopped after http://cgit.drupalcode.org/drupal/commit/?id=d7c496150e822342bfcd0a3004f..., but the problem only hid, but did not fixed.
🔬 2879048-167 @tacituseu tried to restore fail with constructed graph of dependencies based on the collected earlier, but without result.
⏳ pause
⁉ 2879048-169 @tacituseu detected this error again, but with PHP 5.5 and SQLite/Postgresql. It is back after #2871591-167: Allow ComplexData in TypedData to specify computed properties that should be exposed in normalization and other contexts.
⚔ 2921862 @alexpott begins to fight with this problem in a separate issue, since the PostgreSQL failures are too frequent.
📌 As before, we can fix this fails via:
- 2879048-188 additional installation of random modules (does not always work);
- 2879048-189 increase reference count;
- 2879048-224 other change reference count;
👍💎 2921862-18 @alexpott makes a patch that disables gc during non-interactive install phase in tests. This solved the problem of instability with Postgresql (and SQLite, checked in 2879048-178 after commit).
Proposed resolution
Solve the problem, or at least systematize knowledge about it, until the remove PHP 5.6 support (#2842431: [policy] Remove PHP 5.5, 5.6 support in Drupal 8.7).
2879048-129 contains patch, which prevents this behavior. We can use it for php < 7 like workaround.
Also, we can add random modules to change the dependency graph, that get error. See [#2879048-183] patch with random modules. But it looks very strange, and may be necessary in many places in an unpredictable manner. So it is not worth it.
Remaining tasks
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | Postgresql-random-fail.png | 4.04 KB | Anonymous (not verified) |
Comments
Comment #1
Anonymous (not verified) commentedvaplas created an issue. See original summary.
Comment #2
Anonymous (not verified) commented#2879048: Ignore: patch testing issue for #2919863 issue for experiments still actual ;)
Comment #3
Anonymous (not verified) commentedThe fail is back!)
Comment #4
alexpottMaybe be related #2921862: Segfault on PHP5.5 and PostgreSQL - but that one seems consistent.
Comment #5
Anonymous (not verified) commentedI bit updated IS (special "Proposed resolution" section) after last cases with PHP 5.5/SQLite/Postgresql.
#4: Yes, it seems very consistent, but it is random too. Example: https://dispatcher.drupalci.org/job/drupal_patches/36717/console
Comment #6
Anonymous (not verified) commentedIt was classified into #2921862: Segfault on PHP5.5 and PostgreSQL and #2879048: Ignore: patch testing issue for #2919863. Big thanks!
Comment #7
Anonymous (not verified) commented.
Comment #8
tacituseu commentedI think it would be good to leave it open in as a follow-up similar to #2842393: Discover why gc_disable() improves update.php stability as it has good summary of what went on trying to find root cause.
Comment #9
tacituseu commentedComment #10
Anonymous (not verified) commentedI agree. It makes sense! Supplemented the story.
Comment #14
alexpottThis code no longer exists in 8.8.x as we've dropped support for PHP5 so it's unlikely we'll ever get to the bottom of this.