docs(lsphp): correct three comments that overstated what the code does
Review found all three describing behaviour the code does not have: - The range bound does NOT prevent opcache's shared-memory startup failure. With 99-prod-overrides setting interned_strings_buffer=16, a memory_consumption of 8 or 16 is accepted here and still aborts opcache. Documented rather than raising the floor, which would forfeit the superset property. - memory_consumption's 4096 ceiling is ours, not PHP's — PHP imposes no upper bound on that directive. Only the max_accelerated_files range is a vendor clamp. Also records that an out-of-range value resets to PHP's COMPILED default, discarding the image's own override. - The stale-fragment rm -f is defensive, not a bug fix: changing these env vars requires a recreate, which starts from a fresh layer, so the scenario the comment described is not reachable via docker restart. Comments only; no behaviour change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+26
-12
@@ -139,9 +139,15 @@ echo "Container memory: ${CONTAINER_MEMORY_MB}MB | PHP_LSAPI_CHILDREN=${PHP_LSAP
|
|||||||
##
|
##
|
||||||
## Digits-only is what closes the injection: no newline, quote, `$` or `{` can
|
## Digits-only is what closes the injection: no newline, quote, `$` or `{` can
|
||||||
## survive it, so neither an ini-directive injection nor php.ini's `${VAR}`
|
## survive it, so neither an ini-directive injection nor php.ini's `${VAR}`
|
||||||
## interpolation is reachable regardless of what the caller sent. The range
|
## interpolation is reachable regardless of what the caller sent.
|
||||||
## bound is a separate concern — it stops a typo'd value from making opcache
|
##
|
||||||
## fail its shared-memory allocation at startup.
|
## The range bound is a separate, weaker concern: it is a sanity check, NOT a
|
||||||
|
## guarantee that the value works. Measured — with `99-prod-overrides.ini`
|
||||||
|
## setting `opcache.interned_strings_buffer = 16`, a memory_consumption of 8 or
|
||||||
|
## 16 is ACCEPTED here and still aborts opcache at startup ("Insufficient shared
|
||||||
|
## memory for interned strings buffer"), loading no opcache at all. The floor is
|
||||||
|
## not raised to cover that because doing so would forfeit the superset property
|
||||||
|
## below; the panel clamps at 32, well clear of it.
|
||||||
validate_ini_num() {
|
validate_ini_num() {
|
||||||
local name="$1" val="$2" min="$3" max="$4"
|
local name="$1" val="$2" min="$3" max="$4"
|
||||||
INI_NUM=""
|
INI_NUM=""
|
||||||
@@ -257,12 +263,17 @@ EOF
|
|||||||
## `domain`, and the panel is a different repo on a different release cadence.
|
## `domain`, and the panel is a different repo on a different release cadence.
|
||||||
## Validate at the point of use, where the ini is actually generated.
|
## Validate at the point of use, where the ini is actually generated.
|
||||||
##
|
##
|
||||||
## The accepted ranges below are PHP's OWN limits for these directives
|
## The accepted ranges below are deliberately a strict SUPERSET of the panel's
|
||||||
## (opcache refuses memory_consumption under 8 MB and clamps
|
## clamps (32-512 and 2000-32000), so widening a panel clamp later can never
|
||||||
## max_accelerated_files into [200, 1000000]), deliberately a strict SUPERSET
|
## start silently rejecting real sites here.
|
||||||
## of the panel's clamps: a value outside them could not have taken effect
|
##
|
||||||
## anyway, and widening a panel clamp later can never start silently
|
## Provenance, stated honestly: max_accelerated_files [200, 1000000] IS PHP's
|
||||||
## rejecting real sites here.
|
## own clamp. For memory_consumption, 8 is PHP's documented floor but 4096 is
|
||||||
|
## OURS — PHP imposes no upper bound on that directive. It is a typo guard, not
|
||||||
|
## a vendor limit. An out-of-range value is not merely ignored: PHP resets the
|
||||||
|
## directive to its COMPILED default, discarding the image's own
|
||||||
|
## `99-prod-overrides` value, which is a further reason to reject rather than
|
||||||
|
## pass such a value through.
|
||||||
OPCACHE_LINES=()
|
OPCACHE_LINES=()
|
||||||
if [ -n "${OPCACHE_MEMORY_MB:-}" ]; then
|
if [ -n "${OPCACHE_MEMORY_MB:-}" ]; then
|
||||||
validate_ini_num OPCACHE_MEMORY_MB "$OPCACHE_MEMORY_MB" 8 4096
|
validate_ini_num OPCACHE_MEMORY_MB "$OPCACHE_MEMORY_MB" 8 4096
|
||||||
@@ -284,9 +295,12 @@ EOF
|
|||||||
} > "$SCAN_DIR/99-user-opcache.ini"
|
} > "$SCAN_DIR/99-user-opcache.ini"
|
||||||
else
|
else
|
||||||
## Nothing valid to say. Remove rather than leave whatever a previous boot
|
## Nothing valid to say. Remove rather than leave whatever a previous boot
|
||||||
## wrote — the env is the source of truth and the container filesystem
|
## wrote. Defensive only — do not read this as fixing a reachable bug: the
|
||||||
## outlives a `docker restart`. Without this, an override that is later
|
## writable layer does outlive a `docker restart`, but so does the
|
||||||
## cleared (or rejected) would keep applying from the stale fragment.
|
## environment, and changing these vars requires a RECREATE, which starts
|
||||||
|
## from a fresh layer with no stale fragment. Kept because it is free, and
|
||||||
|
## because it makes "no valid override" mean the same thing on every boot
|
||||||
|
## regardless of how the container got here.
|
||||||
rm -f "$SCAN_DIR/99-user-opcache.ini"
|
rm -f "$SCAN_DIR/99-user-opcache.ini"
|
||||||
fi
|
fi
|
||||||
else
|
else
|
||||||
|
|||||||
Reference in New Issue
Block a user