Woven methods keep the aspect instances of the first application for the whole process (tests, Octane, worker mode)

Aperta
  • Qualcosa non va?
#838 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
5/5
Tempo stimato
Più di una settimana
Idoneità per principianti
28/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
php
Ambito
backend, performance, testing

Direzione di ricerca

Start with src/Aop/Framework/The.php:41-43 and the woven InterceptorInjector::forMethod() example in the issue; then compare the three proposed approaches and inspect docs/advices.md and docs/performance.md. The issue requires choosing a design, adding a two-container regression test, measuring the hot path with composer test:performance, and documenting the chosen behavior.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

Bug To Be Discussed

Implementation note: start in plan mode: the hot path is involved, and the fix needs a design decision first.

Found while building goaop/shop-demo-laravel (Laravel 13.40, goaop-laravel-bridge 2.0.0, framework 4.0.0). It affects every integration that creates more than one application or container in one process.

Problem

Every intercepted member keeps its joinpoint in a function-static variable:

static $__joinPoint = InterceptorInjector::forMethod(self::class, 'placeOrder', [
    Interceptor::around(The::aspect(TransactionAspect::class)->aroundTransactional(...)),
], $this->placeOrderOriginalAlias(...));

The::aspect() (src/Aop/Framework/The.php:41-43) resolves the aspect once, on the first call in the process, and the first-class callable binds that instance into the interceptor for good. Aspects are services of the application container (lazy services, a PSR-11 delegate in the Symfony and Laravel integrations), so the instance and its constructor-injected dependencies belong to the application that was current at the first call.

When the process creates another application, the woven code keeps calling the aspect of the first one, with the first application's dependencies:

  • Test suites: Laravel creates a new application for every test; Symfony's KernelTestCase reboots the kernel.
  • Long-running workers: Laravel Octane (scoped services are reset per request), FrankenPHP worker mode, RoadRunner, Swoole, ReactPHP, Symfony's kernel.reset/services_resetter.

Reproduction

The demo's tests/Feature/ShopTest.php without #[RunTestsInSeparateProcesses]:

  1. Test 1 requests /. TransactionAspect is created with application #1's singleton TransactionManager.
  2. Test 3 requests /checkout with application #3. The woven OrderService::placeOrder() runs the aspect of application #1, which commits on application #1's TransactionManager. The controller reads application #3's manager: Failed asserting that 0 is identical to 50.

Run alone, or with every test in its own process, the test passes. The demo now runs its tests in separate processes. The bridge's and the bundle's own test suites do the same ("Tests touching the kernel must run in separate PHPUnit processes"), so the integrations' tests never see it.

Impact

  • Application test suites that assert on what aspects did, or on state the aspects share with the application, fail from the second test on. Process isolation makes them several times slower.
  • In worker mode, an aspect that depends on a request-scoped service (the current user, a per-request unit of work, a scoped Octane binding) keeps the first request's instance for the lifetime of the worker. That's a correctness and security problem: one request's state leaks into the next.
  • docs/performance.md ("Long-running workers") presents joinpoints staying initialized between requests as a benefit, without this caveat.

Options for the plan

  1. Container generation: the aspect container (or The) carries a generation that integrations bump when they attach a new delegate or reset services (setDelegate(), Octane's RequestReceived, kernel.reset). Interceptors keep the aspect id and re-resolve the aspect callable when the generation changed, with one int comparison per call on the hot path. Measure it with composer test:performance.
  2. Reset hook: a framework API that clears the static joinpoints of all woven classes, e.g. a registry of joinpoints filled by InterceptorInjector::for*() whose interceptors are rebuilt on the next call. Integrations call it between applications (testbench tearDown, Octane listener, kernel.reset).
  3. Documentation only: state that aspects are process singletons. Aspects must not keep request-scoped dependencies; inject a container or a closure instead and resolve them per call. Tests that need fresh aspects run in separate processes.

Acceptance

  • A test that creates two aspect containers (two applications) in one process: the second application's advice calls reach the second application's aspect instance.
  • composer test:performance before and after: no measurable regression of the joinpoint hot path.
  • Documentation of the chosen behaviour (docs/advices.md, docs/performance.md, and the bridge/bundle READMEs for the reset hook).
Lingua principale
PHP
Stelle
1.7k
Fork
166
Merge medio
1g 1h
PR unite (30g)
38

Preparare l'ambiente

Questo progetto non fornisce container di sviluppo, Dockerfile né guida per i contributori, quindi l'ambiente è a tuo carico: parti dal suo README e consulta la nostra guida al primo contributo per i passaggi generali.

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di goaop/framework

Tutte le issue di goaop/framework

Issue simili

Altre issue su PHP

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.