[13.x] Bound error page query listener to prevent memory bloat in Octane#59309
Merged
taylorotwell merged 2 commits intolaravel:13.xfrom Mar 23, 2026
Merged
[13.x] Bound error page query listener to prevent memory bloat in Octane#59309taylorotwell merged 2 commits intolaravel:13.xfrom
taylorotwell merged 2 commits intolaravel:13.xfrom
Conversation
|
Thanks for submitting a PR! Note that draft PRs are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface. Pull requests that are abandoned in draft may be closed due to inactivity. |
…kers Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
fc3ccb7 to
f674878
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #56652
The exception renderer's
Listenerstores unbounded query data (full SQL strings and all bindings) that its consumer — the debug error page — can never meaningfully display. For bulk INSERT statements, a single stored query can exceed 500KB of SQL text with 18,000+ bindings. With 100 queries stored, this causes Octane workers to allocate 140MB+ of memory that PHP's allocator never returns to the OS.Scope & Safety
This change only modifies
Listener::onQueryExecuted()in the exception renderer. It does not affect:APP_DEBUG=true(FoundationServiceProvider:74). When debug is off, this code never runs.QueryExecutedevent. They never touch the Renderer Listener.DB::listen()/DB::getQueryLog()— Separate systems on theConnectionclass, unmodified.strlencheck and passes through completely untouched.Root Cause
Three compounding issues in
Listener::onQueryExecuted():No size limit on stored SQL — A bulk INSERT with 6,000 rows generates ~500KB of SQL text. The error page renders this in a
max-h-32 overflow-hiddencontainer where only a few hundred characters are visible.No limit on stored bindings — The same INSERT stores 18,000 bindings. The error page's
applicationQueries()substitutes these via per-bindingpreg_replace('/?/', ...)on the full SQL string — O(n²) complexity that would freeze the browser long before it renders.Off-by-one in query cap —
=== 101stores 101 entries but the template says "only the first 100 queries are displayed".The memory IS freed between Octane requests (
memory_get_usage(false)drops back to baseline), but PHP's memory allocator holds the OS pages, inflating worker RSS permanently. The remaining ~22MB idle after the fix is PHP's allocator retaining pages from the bulk array operations in userland code — this is standard PHP behavior unrelated to the Listener.The Fix
Three targeted changes to
Listener::onQueryExecuted(), no other files modified:mb_strcutfor UTF-8 safety). Normal queries pass through untouched. The error page's CSS overflow already handles visual truncation.substr_count($sql, '?')so bindings always correspond to the?placeholders — no dangling unsubstituted placeholders on the error page.>= 100instead of=== 101to match the template's "first 100" message.Measured Results
With Octane
Tested on Laravel 13 + Octane (FrankenPHP), single worker,
APP_DEBUG=true, inserting 630,000 rows (105 batches × 6,000 rows) per request:For Octane workers running in local development, this prevents a single bulk insert request from permanently inflating the worker's memory footprint by 120MB+.
Without Octane
Without Octane (Herd,
php artisan serve, PHP-FPM), each request is a fresh process so memory is reclaimed when the process exits. However, the fix still reduces peak memory during a request:This means fewer
Allowed memory size exhaustederrors for applications with conservativememory_limitsettings.Testing & Edge Cases
Unit tests (8 tests, 23 assertions)
test_queries_returns_expected_shapetest_listener_caps_at_100_queriestest_large_sql_is_truncatedtest_bindings_match_placeholder_count?count in truncated SQLtest_excess_bindings_trimmedtest_short_sql_and_bindings_not_modifiedtest_query_with_no_bindings_unchangedSELECT count(*)→ stored exactlytest_normal_query_skips_truncationEdge cases considered
?countmb_strcutensures clean truncation at character boundary?inside string literals in SQLsubstr_countmay overcount — butapplicationQueries()has the same pre-existing behavior?placeholdersAPP_DEBUG=falseVisually verified
The debug error page was inspected in Chrome for:
How to Reproduce & Test
Setup
composer create-project laravel/laravel memory-test cd memory-test composer require laravel/octane php artisan octane:install --server=frankenphp php artisan migrateAdd test route to
routes/web.phpRun
Unit tests
Known Limitations
?placeholders in the truncated SQL are not stored. This is preferable to the current behavior whereapplicationQueries()would take minutes to render 18,000 bindings via O(n²)preg_replace.memory_get_usage(true)between requests — that's PHP's allocator holding OS pages, which is expected and standard for any long-lived PHP process. It reduces the peak allocation so fewer pages are claimed in the first place.Prior Art
Three previous PRs attempted to fix this issue:
gc_collect_cycles()(closed)This PR takes a narrower approach: the Listener should not store data its consumer cannot display. The error page cannot render 500KB SQL strings (CSS overflow clips them) and cannot efficiently substitute 18,000 bindings (O(n²)
preg_replace). Bounding the stored data to what's displayable is a design correction, not a workaround.