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

Abierto
  • ¿Algo no está bien?
#838 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 1 día

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
5/5
Tiempo estimado
Más de una semana
Aptitud para principiantes
28/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
php
Área
backend, performance, testing

Línea de trabajo

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.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

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).
Lenguaje dominante
PHP
Estrellas
1.7k
Forks
166
Merge medio
1 d 1 h
PR fusionados (30 d)
38

Preparar el entorno

Este proyecto no incluye contenedor de desarrollo, Dockerfile ni guía de contribución, así que la configuración corre por tu cuenta: empieza por su README y consulta nuestra guía para la primera contribución para los pasos generales.

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de goaop/framework

Todos los issues de goaop/framework

Issues similares

Más issues de PHP

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.