Skip to content

[Console] Fix tree helper traversal bugs - #1

Closed
arena-ai-coding-agent[bot] wants to merge 1 commit into
7.4from
arena/01a0d6fc-symfony2
Closed

[Console] Fix tree helper traversal bugs#1
arena-ai-coding-agent[bot] wants to merge 1 commit into
7.4from
arena/01a0d6fc-symfony2

Conversation

@arena-ai-coding-agent

@arena-ai-coding-agent arena-ai-coding-agent Bot commented Sep 25, 2026

Copy link
Copy Markdown

Summary

  • Preserve TreeNode instances passed through TreeNode::fromValues().
  • Avoid evaluating lazy child generators twice during traversal.
  • Detect cycles on the current ancestor path while allowing shared nodes in separate branches.
  • Add regression tests for these cases.

Testing

  • git diff --check
  • PHPUnit could not run because PHP is not installed in the environment.

sadiqk2 pushed a commit that referenced this pull request Oct 29, 2026
@nicolas-grekas
bug symfony#66597 [Lock] Fix FlockStore keeping a closed handle after…
a639d60 
… a failed promotion (nicolas-grekas)

This PR was merged into the 6.4 branch.

Discussion
----------

[Lock] Fix FlockStore keeping a closed handle after a failed promotion

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

When promoting a read lock fails, FlockStore closes its file handle but keeps it in the state of the key: `isAcquired()` then returns true, and `release()` or any later `acquire()` throws `TypeError: flock(): Argument #1 ($stream) must be an open stream resource`.

```php
$lock->acquireRead();
$lock->acquire(); // false: another process holds a read lock too
$lock->release(); // TypeError
```

Since flock() releases the current lock before converting it, the state is now removed before the conversion. This also covers the exit paths that symfony#66526 adds.

Commits
-------

a793805 [Lock] Fix FlockStore keeping a closed handle after a failed promotion
sadiqk2 pushed a commit that referenced this pull request Oct 29, 2026
@nicolas-grekas
bug symfony#66662 [HttpKernel] Fix HttpCache serving a 500 instead of…
f432353 
… a 503 for stale ESI entries when the cache is locked (lyrixx)

This PR was merged into the 6.4 branch.

Discussion
----------

[HttpKernel] Fix HttpCache serving a 500 instead of a 503 for stale ESI entries when the cache is locked

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

When a stale cache entry is locked by another request because the backend is slow, `HttpCache::lock()` waits for the lock. It then gives up and serves a 503 "anti dog-pile" response built from the cached entry:

```php
$entry->setStatusCode(503);
$entry->setContent('503 Service Unavailable');
$entry->headers->set('Retry-After', 10);
```

If the entry contains ESI/SSI includes, it also has an `X-Body-Eval` header. `restoreResponseBody()` then parses `'503 Service Unavailable'` (23 chars) as an eval body split by 24-char boundaries, and `strpos()` throws:

```
ValueError: strpos(): Argument symfony#3 ($offset) must be contained in argument #1 ($haystack)
```

This happens in `HttpCache`, outside of `HttpKernel::handle()`, so the client gets a generic 500 error page instead of a 503 with `Retry-After`.

We hit this in production: the homepage was cached with 3 `render_esi` fragments and the database server was unreachable. The request holding the lock waited on the DB, and every concurrent request got this 500 after exactly 5 seconds (the `waitForLock()` timeout).

For entries without ESI, the `'503 Service Unavailable'` text was never sent anyway: `restoreResponseBody()` overwrites it with the stale body loaded from `X-Body-File`. `testDegradationWhenCacheLocked()` checks exactly that: old response, 503 status.

This PR only sets the text when the entry has no body to restore (e.g. a custom `StoreInterface` returning a fully loaded `Response`). Stale ESI entries are then served like the other ones: old response with its fragments restored, 503 status and `Retry-After` header. Restoring the fragments triggers their sub-requests through the surrogate, as the `stale-while-revalidate` path of the same method already does.

The bug was introduced in symfony#50244 (3c49177). Before it, `restoreResponseBody()` included the `X-Body-File` set by the Store and ignored the content set by `lock()`, so stale ESI entries were served with their fragments, as they are again now.

Commits
-------

b0af9e9 [HttpKernel] Fix HttpCache serving a 500 instead of a 503 for stale ESI entries when the cache is locked
@sadiqk2 sadiqk2 closed this Oct 29, 2026
sadiqk2 pushed a commit that referenced this pull request Oct 29, 2026
@nicolas-grekas
bug symfony#67376 [DependencyInjection] Preserve XML schema temporary…
50fbc47 
… files in forked processes (mbeccati)

This PR was merged into the 6.4 branch.

Discussion
----------

[DependencyInjection] Preserve XML schema temporary files in forked processes

| Q | A |
|---|---|
| Branch? | 6.4 |
| Bug fix? | yes |
| New feature? | no |
| Deprecations? | no |
| Issues | N/A |
| License | MIT |

XmlFileLoader registers a shutdown callback to remove its temporary schema file. Forked children inherit this callback and delete the file when they exit, although the parent process still needs it.

Restrict cleanup to the process that created the file, using a PID guard. A regression test verifies that the file survives a child process exiting.

The test fails before the fix and passes afterward. The full DependencyInjection suite passes: 1,631 tests, 16 skipped. Style checks also pass.

Merge-up to 7.4: convert the regression test’s PHPUnit annotations to attributes.

References:
```
Fatal error: Uncaught PHPUnit\Event\Code\NoTestCaseObjectOnCallStackException: Cannot find TestCase object on call stack in /home/runner/work/php-latest-builds/php-latest-builds/.phpunit/phpunit-13-0/src/Util/Test.php:39
Stack trace:
#0 /home/runner/work/php-latest-builds/php-latest-builds/.phpunit/phpunit-13-0/src/Event/Value/Test/TestMethodBuilder.php(60): PHPUnit\Util\Test::currentTestCase()
#1 /home/runner/work/php-latest-builds/php-latest-builds/.phpunit/phpunit-13-0/src/Runner/ErrorHandler.php(214): PHPUnit\Event\Code\TestMethodBuilder::fromCallStack()
symfony#2 [internal function]: PHPUnit\Runner\ErrorHandler->__invoke(2, 'unlink(/tmp/sym...', '/home/runner/wo...', 820)
symfony#3 /home/runner/work/php-latest-builds/php-latest-builds/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php(820): unlink('/tmp/symfonylth...')
symfony#4 [internal function]: Symfony\Component\DependencyInjection\Loader\XmlFileLoader::{closure:Symfony\Component\DependencyInjection\Loader\XmlFileLoader::shouldEnableEntityLoader():819}()
symfony#5 {main}
  thrown in /home/runner/work/php-latest-builds/php-latest-builds/.phpunit/phpunit-13-0/src/Util/Test.php on line 39
```

e.g. in: https://github.com/mbeccati/php-latest-builds/actions/runs/39962411029/job/120443519933

Commits
-------

e5f56d9 [DependencyInjection] Preserve XML schema temporary files in forked processes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@sadiqk2