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 it

Also 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:

👍💎 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

CommentFileSizeAuthor
#5 Postgresql-random-fail.png4.04 KBvaplas
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

vaplas created an issue. See original summary.

vaplas’s picture

vaplas’s picture

Issue summary: View changes

The fail is back!)

alexpott’s picture

Maybe be related #2921862: Segfault on PHP5.5 and PostgreSQL - but that one seems consistent.

vaplas’s picture

Title: Random fail with PHP 5.6 (gc collecting cycles?) (DrupalCI-only?) » Random fail with PHP 5.5 and PHP 5.6 (gc collecting cycles?) (DrupalCI-only?)
Issue summary: View changes
FileSize
4.04 KB

I 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
postgresql random fail
vaplas’s picture

Title: Random fail with PHP 5.5 and PHP 5.6 (gc collecting cycles?) (DrupalCI-only?) » Random fail with PHP 5.5 and PHP 5.6 due to bug with gc collecting cycles
Category: Bug report » Support request
Issue summary: View changes
Status: Active » Fixed
Related issues: +#2921862: Segfault on PHP5.5 and PostgreSQL, +#2842393: Discover why gc_disable() improves update.php stability
vaplas’s picture

.

tacituseu’s picture

Title: Random fail with PHP 5.5 and PHP 5.6 due to bug with gc collecting cycles » Discover why gc_disable() improves tests stability during non-interactive install
Category: Support request » Task
Status: Fixed » Postponed

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

tacituseu’s picture

Title: Discover why gc_disable() improves tests stability during non-interactive install » Discover why gc_disable() during non-interactive install improves tests stability
Status: Postponed » Active
vaplas’s picture

Issue summary: View changes

I agree. It makes sense! Supplemented the story.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.