Skip to content

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

Merged
DeusData merged 1 commit into
DeusData:mainfrom
muba00:fix/php-lsp-trait-self-use-oom
Jul 9, 2026
Merged

fix(lsp/php): prevent OOM when a PHP trait uses itself#920
DeusData merged 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.

@DeusData DeusData added this to the 0.9.1-rc milestone Jul 8, 2026
@DeusData DeusData merged commit 424e4dc into DeusData:main Jul 9, 2026
19 checks passed
@DeusData

DeusData commented Jul 9, 2026

Copy link
Copy Markdown
Owner

Merged — thank you for an outstanding report and fix. The differential reduction down to the 4-line reproducer, the two-layer fix (semantic short-circuit + snapshotted loop bound), and the confirmed-red-first regression test are model contributions. This closes a genuinely nasty OOM — much appreciated.

jfischer-motivity pushed a commit to jfischer-motivity/codebase-memory-mcp that referenced this pull request Jul 9, 2026
Two reported indexer OOMs reduce to the same self-flattening loop the
trait-uses-itself fix closed (self-flatten short-circuit + snapshotted
loop bound in flatten_trait_into_class):

- a trait whose name collides with an aliased `use ... as` import it
  composes (DeusData#765's exact single-file reproducer) — the alias
  canonicalizes onto the enclosing trait via the short-name fallback
- a class sharing its short name with an aliased trait import (DeusData#951's
  reproducer, reduced to one file: the trait definition being
  unavailable is what forces the short-name fallback onto the class)

Both fixtures were verified RED against the pre-guard code (each hangs
the suite within the 90s alarm when the two guards are reverted, each
independently as the first test run) and GREEN with the guards in
place. Landing them as permanent regression guards; the issues close as
fixed-by-the-DeusData#920-pair.

Closes DeusData#765
Closes DeusData#951

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
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