RTD collision with opcache

Bug #1968228 reported by Matt Coleman
22
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Symfony
Fix Released
Unknown
php
Unknown
Unknown
php7.4 (Ubuntu)
Invalid
Undecided
Athos Ribeiro
Focal
Fix Released
Undecided
Athos Ribeiro

Bug Description

[Impact]

As explained in https://www.php.net/manual/en/intro.opcache.php,

OPcache improves PHP performance by storing precompiled bytecode in shared memory, removing the need to load and parse scripts on each request.

A bug in PHP OPcache code (https://bugs.php.net/bug.php?id=79603) may make PHP crash when this feature is enabled. For instance, this has been observed with Symfony's caching feature.

The fix is available upstream at https://github.com/php/php-src/commit/4f47ba99f002d50e11c111b8625d81f79b2bf52f

[Test Plan]

Use the following script to reproduce the issue on a focal environment with php7.4 installed:

BEGIN_REPRODUCER
#!/bin/bash

PHP_TMP_DIR="$(mktemp -d)"
PHP_CACHE_DIR="${PHP_TMP_DIR}/cache"
PHP_FILE1="${PHP_TMP_DIR}/file1.php"
PHP_FILE2="${PHP_TMP_DIR}/file2.php"
mkdir -p "${PHP_CACHE_DIR}"

# prints information about the current php environment
# php -d opcache.enable_cli=1 -d opcache.file_cache_only=1 -d opcache.file_cache="${PHP_CACHE_DIR}" -i

cat > "${PHP_FILE1}" <<EOF
<?php
return function() {};
EOF

cat > "${PHP_FILE2}" <<EOF
<?php
\$file = '${PHP_FILE1}';
var_dump(include \$file);
touch(\$file);
var_dump(include \$file);
EOF

sleep 5
php -d opcache.enable_cli=1 -d opcache.file_cache_only=1 -d opcache.file_cache="${PHP_CACHE_DIR}" ${PHP_FILE1}
php -d opcache.enable_cli=1 -d opcache.file_cache_only=1 -d opcache.file_cache="${PHP_CACHE_DIR}" ${PHP_FILE2}

# clean up
rm -rf "${PHP_TMP_DIR}"
END_REPRODUCER

Re-run the reproducer after installing the proposed fix to ensure the new package is no longer affected.

[Where problems could occur]

While the patch being proposed seems to be straightforward, it was applied on top of two other patches in Zend/zend_compile.c and will be back-ported (it does apply on different lines without changes). This may lead to unpredicted code-paths being executed, triggering possible bugs not seen upstream.

The set of patches is small enough and if needed, we could apply the whole set to make the codepaths closer to what upstream intended for PHP 7.4.7.

$ git log --oneline PHP-7.4.3..PHP-7.4.7 -- Zend/zend_compile.c
4f47ba99f0 Fix bug #79603, by retrying on RTD key collision
c5159b3832 Check asserts early
2dddab01ae Avoid "Anonymous class wasn't preloaded" error by lazely loading of not preloaded part of a preloaded script

Moreover, recompiling PHP linking it to possible new versions of libraries that may have been SRU'd could trigger unwanted behaviors which were not observed before.

[Other Info]

This bug was fixed in php 7.4.7 and is not present in Ubuntu releases other than focal.

[Original message]

As described in PHP bug #79603, it is possible to cause opcache collisions that result in a fatal error.

https://bugs.php.net/bug.php?id=79603

It has been fixed in this upstream commit:
https://github.com/php/php-src/commit/4f47ba99f002d50e11c111b8625d81f79b2bf52f

The changes to zend_compile.c apply cleanly, just at different line numbers.

This has been reproduced on Ubuntu 20.04 with version 7.4.3-4ubuntu2.8 of php7.4-opcache installed.

Related branches

Revision history for this message
Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in php7.4 (Ubuntu):
status: New → Confirmed
Revision history for this message
Lena Voytek (lvoytek) wrote :

Hello,

Thank you for taking the time to make this bug report. I can confirm this would be a straightforward fix for 20.04. I was unable to reproduce it personally using the test case provided in the PHP bug report. Is there a specific way you have been able to reproduce it?

Thanks

Utkarsh Gupta (utkarsh)
tags: added: server-todo
Utkarsh Gupta (utkarsh)
tags: added: cpc-newcomer
Changed in php7.4 (Ubuntu):
assignee: nobody → Athos Ribeiro (athos-ribeiro)
Revision history for this message
Matt Coleman (mcoleman) wrote :

This issue happens intermittently on FPM servers and Symfony's caching functionality is the only place where I've seen it occur. Unfortunately, it's just something that I've seen in logs; I don't have a quick script that simply reproduces the issue. I added the Symfony bug on GitHub (which was closed due to it being a bug in PHP, not Symfony) to this ticket's "Affects" section, above.

Changed in symfony:
status: Unknown → Fix Released
Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote (last edit ):

Minimal reproducer:

#!/bin/bash
# prints information about the current php environment
# php -d opcache.enable_cli=1 -d opcache.file_cache_only=1 -d opcache.file_cache="/root/php-cache" -i
php -d opcache.enable_cli=1 -d opcache.file_cache_only=1 -d opcache.file_cache="/root/php-cache" file1.php
php -d opcache.enable_cli=1 -d opcache.file_cache_only=1 -d opcache.file_cache="/root/php-cache" file2.php

Expected result:

object(Closure)#1 (0) {
}
object(Closure)#1 (0) {
}

Actual (buggy) result:

object(Closure)#1 (0) {
}
PHP Fatal error: Runtime definition key collision for function {closure}. This is a bug in /root/file1.php on line 2

description: updated
Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote :
Revision history for this message
Matt Coleman (mcoleman) wrote :

The package in your PPA fixes the issue. Thanks, Athos!

Changed in php7.4 (Ubuntu):
status: Confirmed → In Progress
Revision history for this message
Robie Basak (racb) wrote : Proposed package upload rejected

An upload of php7.4 to focal-proposed has been rejected from the upload queue for the following reason: "See previous comment".

Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote :

The upload was rejected due to the discussion in LP: #1890263

Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote :

I split the SRU proposals and re-uploaded this one as a standalone one

Revision history for this message
Robie Basak (racb) wrote : Please test proposed package

Hello Matt, or anyone else affected,

Accepted php7.4 into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/php7.4/7.4.3-4ubuntu2.11 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in php7.4 (Ubuntu Focal):
status: New → Fix Committed
tags: added: verification-needed verification-needed-focal
Changed in php7.4 (Ubuntu):
status: In Progress → Invalid
Changed in php7.4 (Ubuntu Focal):
assignee: nobody → Athos Ribeiro (athos-ribeiro)
Revision history for this message
Matt Coleman (mcoleman) wrote :

I installed the package from focal-proposed and confirmed that the synthetic test provided in #4 runs properly, without any errors. Real-world extended validation tests to follow...

Revision history for this message
Ubuntu SRU Bot (ubuntu-sru-bot) wrote : Autopkgtest regression report (php7.4/7.4.3-4ubuntu2.11)

All autopkgtests for the newly accepted php7.4 (7.4.3-4ubuntu2.11) for focal have finished running.
The following regressions have been reported in tests triggered by the package:

phpmyadmin-sql-parser/4.6.1-2 (s390x)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/focal/update_excuses.html#php7.4

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

Revision history for this message
Matt Coleman (mcoleman) wrote :

The proposed update is installed on the machines in an internal lab that's running nightly automated end-to-end system tests. I'll follow up on Monday with the results from over the weekend.

Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote :

Hi Matt,

Thanks for checking this one out. Please, let us know when you are done verifying the fix.

Regarding the failing test for phpmyadmin-sql-parser/4.6.1-2 (s390x)

https://autopkgtest.ubuntu.com/packages/p/phpmyadmin-sql-parser/focal/s390x shows that the test also failed for the migration reference on a re-trigger. This is a flaky test.

Revision history for this message
Matt Coleman (mcoleman) wrote :

Hi Athos,

The issue has not occurred in our system test environment since installing the package. The new package looks good!

Thanks for clarifying that about the failing s390x test: I was completely at a loss as to how this could have caused that particular failure.

Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote :

Based on Matt's verification, and in the test script described above, whose results are detaled below, I am marking the verification as done. As mentioned above, the failure for phpmyadmin-sql-parser/4.6.1-2 (s390x) is due to a flaky test and was already failing before this change was introduced.

From a focal machine, I ran the reproducer script. Output:

# ./reproducer.sh
object(Closure)#1 (0) {
}
PHP Fatal error: Runtime definition key collision for function {closure}. This is a bug in /tmp/tmp.ThHJm8SrKz/file1.php on line 2

Then, I installed the php7.4 and php7.4-cli packages from focal-proposed and re-ran the reproducer. Output:

# ./reproducer.sh
object(Closure)#1 (0) {
}
object(Closure)#1 (0) {
}

Which confirms the fix.

tags: added: verification-done verification-done-focal
removed: verification-needed verification-needed-focal
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package php7.4 - 7.4.3-4ubuntu2.11

---------------
php7.4 (7.4.3-4ubuntu2.11) focal; urgency=medium

  * d/p/0048-Fix-bug-79603-by-retrying-on-RTD-key-collision.patch: retry on RTD
    key collision. (LP: #1968228)

 -- Athos Ribeiro <email address hidden> Thu, 05 May 2022 21:16:42 -0300

Changed in php7.4 (Ubuntu Focal):
status: Fix Committed → Fix Released
Revision history for this message
Brian Murray (brian-murray) wrote : Update Released

The verification of the Stable Release Update for php7.4 has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.