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

オープン
  • 何か問題がありますか？
#838 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

まだ誰も着手していません。

評価

難易度
5/5
見積もり時間
1週間以上
初心者へのやさしさ
28/100
issue の種類
バグ
明瞭さ
明確に書かれている
活発さ
活発
技術スタック
php
領域
backend, performance, testing

調査の方向性

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.

索引モデルが issue の本文から書いたものです。

説明

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).
主要言語
PHP
スター
1.7k
フォーク
166
平均マージ
1日 1時間
マージ済み PR（30日）
38

環境構築

このプロジェクトには開発コンテナ、Dockerfile、コントリビューションガイドがありません。まず README を読み、一般的な手順ははじめてのコントリビューションガイドを参照してください。

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

goaop/framework のほかの issue

goaop/framework の issue をすべて見る

似ている issue

PHP の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。