php / php/php-src

opcache.dups_fix is honored for duplicate classes but not duplicate functions

Open
#22,214 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Bug Extension: opcache Status: Needs Triage
Dominant language
C
Stars
40.4k
Forks
8.1k
Avg merge
2d 13h
Merged PRs (30d)
96

Description

Description

opcache.dups_fix is documented as a fix for "Cannot redeclare" errors, but it only covers classes, not functions.

Inside opcache the directive is read in the table-copy that installs a cached script's symbols (ext/opcache/zend_accelerator_util_funcs.c). The class-table copy honors it — when ignore_dups is set it keeps the existing class and skips the duplicate:

/* _zend_accel_class_hash_copy */
} else if (UNEXPECTED(!ZCG(accel_directives).ignore_dups)) {
    ...
    zend_class_redeclaration_error(E_ERROR, Z_PTR_P(t));
    return;
}
continue; /* ignore_dups: keep the first definition */

The function-table copy right next to it doesn't check the directive at all — it goes straight to the fatal:

/* _zend_accel_function_hash_copy */
t = zend_hash_find_known_hash(target, p->key);
if (UNEXPECTED(t != NULL)) {
    goto failure; /* -> "Cannot redeclare function ..." regardless of opcache.dups_fix */
}

So with opcache.dups_fix=1 a duplicate class is tolerated (first wins) but a duplicate function still fatals. The directive name and docs don't distinguish between the two, so this reads like an oversight rather than something intentional.

Where it bites

Long-running application servers that re-execute require_once'd files per request (we ran into this building ZealPHP, an OpenSwoole-based runtime). opcache re-installs a cached script's symbols into a table that already has them; dups_fix covers the class collision, but the function collision still kills the request with "Cannot redeclare function". So dups_fix only half-solves it for these setups — WordPress for example gets past its class redeclares with dups_fix=1 but then dies on the first function (_wp_can_use_pcre_u in wp-includes/compat.php).

Suggested fix

Make the function copy consistent with the class copy:

 		t = zend_hash_find_known_hash(target, p->key);
 		if (UNEXPECTED(t != NULL)) {
-			goto failure;
+			/* Honor opcache.dups_fix for functions too — the class-table
+			 * copy above already does. Keep the first-declared function. */
+			if (!ZCG(accel_directives).ignore_dups) {
+				goto failure;
+			}
+			continue;
 		}

I've tested this against 8.4 and it does the job (WordPress runs clean under opcache + a per-request re-execution model with it). Happy to open a PR with a .phpt if the asymmetry is agreed to be unintended — mostly wanted to check whether it's deliberate before sending one.

PHP Version

PHP 8.4 (the code is the same on master)

Operating System

Linux

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in ext/opcache/zend_accelerator_util_funcs.c and compare _zend_accel_function_hash_copy with _zend_accel_class_hash_copy, focusing on how ignore_dups is handled. Add a .phpt test for duplicate functions with opcache.dups_fix enabled and verify that duplicates are tolerated there while the directive-disabled case still reports the redeclaration error.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, php
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.