Skip to content

fix(lsp/php): prevent OOM when a PHP trait uses itself#920

Open
muba00 wants to merge 1 commit into
DeusData:mainfrom
muba00:fix/php-lsp-trait-self-use-oom
Open

fix(lsp/php): prevent OOM when a PHP trait uses itself#920
muba00 wants to merge 1 commit into
DeusData:mainfrom
muba00:fix/php-lsp-trait-self-use-oom

Conversation

@muba00

@muba00 muba00 commented Jul 6, 2026

Copy link
Copy Markdown

What does this PR do?

Fixes an unbounded memory blow-up (observed 40 GB+, freezing the host)
when the PHP Hybrid-LSP pass indexes a file whose trait uses itself. It was
hit in a real project by a 4-line trait; the only workaround was to add the file
to .cbmignore.

Minimal reproducer (index_repository, mode: "full" — runs away on main):

<?php
namespace App;
trait EnumTrait {
    use EnumTrait;                                  // trait adopts itself
    public function getRandom(): int { return 1; }
}

It also triggers via an alias that resolves back to the enclosing trait by short
name, which is how it appeared in the wild (use Vendor\EnumTrait as X; use X;
inside a trait that is itself named EnumTrait).

Root cause

flatten_trait_into_class() in internal/cbm/lsp/php_lsp.c copies a trait's
methods onto the using class by iterating the type registry with a live upper
bound
:

for (int i = 0; i < reg->func_count; i++) {         // bound grows inside the loop
    if (strcmp(src->receiver_type, canonical_trait_qn) != 0) continue;
    ...
    rf.receiver_type = class_qn;                     // copy tagged with the user
    cbm_registry_add_func(reg, rf);                  // APPENDS → func_count++
}

cbm_registry_add_func() appends to the very array being iterated (no dedup) and
increments reg->func_count. Normally the copies get receiver_type = class_qn,
which differs from the trait, so they don't re-match and the loop ends.

When a trait adopts itself, class_qn == canonical_trait_qn, so every copied
method is tagged with the trait's own QN and re-matches the loop filter
appending another method and pushing the bound out ahead of i. The loop never
terminates, arena-allocating a fresh method each pass until memory is exhausted.

(Confirmed by differential reduction — self-adoption + ≥1 member + a trait use
are each necessary — and a live sample(1) showing flatten_trait_into_class → cbm_arena_sprintf with the footprint climbing through 6 GB.)

Fix (internal/cbm/lsp/php_lsp.c)

  1. Short-circuit self-flattening — a trait cannot meaningfully use itself
    (PHP raises "Trait X cannot use itself"), so return early when
    class_qn == canonical_trait_qn. This removes the only path by which an
    appended func can re-match the filter.
  2. Snapshot the loop bound before iterating, so entries appended during the
    loop are never revisited — closing the mutate-while-iterating hazard
    structurally, independent of (1).

Checklist

  • Every commit is signed off (git commit -s) — DCO trailer present
  • Tests pass locally — full build/c/test-runner (ASan+UBSan): 5916 passed,
    1 skipped, 0 failures, 0 sanitizer errors
    ; php_lsp suite 279/279
  • Lint passes (make -f Makefile.cbm lint-ci) — could not run locally
    (clang-format / clang-tidy / cppcheck not installed). Diff manually checked
    against .clang-format (LLVM, ColumnLimit 100: added lines ≤ 79, 4-space
    indent, attach braces) and mirrors the surrounding formatted code; please
    let CI verify.
  • New behavior is covered by a test (reproduce-first) — added
    phplsp_trait_self_use_terminates, confirmed it hangs/OOMs on pre-fix
    code
    and passes after the fix.

Verified end to end: the production binary now indexes the original triggering
file (and its real-world siblings) in < 0.5 s at ~6 MB RSS instead of running
away.

flatten_trait_into_class() iterated reg->funcs with a live upper bound
(reg->func_count) while cbm_registry_add_func() appended to that same
array inside the loop. Each copied method is tagged with
receiver_type = class_qn, so when a trait adopts itself -- directly
(`trait T { use T; }`) or via an alias that resolves back to the trait
by short name (`use X\T as A; use A;`) -- class_qn equals
canonical_trait_qn and every appended method re-matches the loop filter,
extending the iteration. The loop never terminates: it arena-allocates a
fresh method each pass until the process exhausts all memory (observed:
40 GB+, freezing the host).

Fix:
- Short-circuit self-flattening: a trait cannot meaningfully use itself
  (PHP itself rejects it), so return early when class_qn equals the
  resolved trait QN.
- Snapshot reg->func_count before the loop so entries appended during
  iteration are never revisited (defensive against the mutate-while-
  iterating hazard in general).

Add regression test phplsp_trait_self_use_terminates: it hangs/OOMs on
the pre-fix code and passes after. Verified end to end -- the prod binary
now indexes the triggering file in <0.5s at ~6 MB RSS instead of running
away.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Mubariz <mubariz@sportimport.de>
@muba00 muba00 requested a review from DeusData as a code owner July 6, 2026 16:25
@DeusData DeusData added bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges stability/performance Server crashes, OOM, hangs, high CPU/memory priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. labels Jul 7, 2026
@DeusData

DeusData commented Jul 7, 2026

Copy link
Copy Markdown
Owner

Thanks for the PHP trait OOM fix and the minimal reproducer. Triage: high-priority parsing/stability bug.

Review focus: the trait-copy loop must use a stable bound or visited guard so self-use and alias-back-to-self cases cannot grow the registry unboundedly, while ordinary trait flattening still works. The real-world OOM impact makes this release-sensitive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. stability/performance Server crashes, OOM, hangs, high CPU/memory

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants