[pull] canary from vercel:canary - #1407
Merged
Merged
Conversation
### What? Adds a CI job that runs selected Rust unit tests under [Miri](https://github.com/rust-lang/miri) to detect undefined behavior in unsafe code. The job currently covers the low-level crates that are compatible with Miri: - `turbo-persistence` - `turbo-rcstr` - `turbo-tasks-malloc` It also makes those crates Miri-compatible by: - using provenance-free tagged-pointer operations in `turbo-rcstr`; - disabling mimalloc and native compression under Miri; - making mmap support a default-enabled Cargo feature that is disabled for Miri and wasm; - running persistence through file I/O when mmap is disabled; - skipping only tests measured to exceed the Miri time budget; - removing leaked test arenas from the analyzer predicate tests. ### Why? Miri can detect invalid memory access and other undefined behavior that normal Rust tests may not expose. Running it in CI gives low-level unsafe code an additional correctness check. The job uses an explicit package allowlist because Turbo Tasks builds its generated registry from linker sections, and Miri does not support the linker-defined section symbols required by that registry. ### How? - Installs the `miri` Rust component in CI and the development container. - Adds a dedicated `test-cargo-unit-miri` task and reusable workflow configuration. - Makes `memmap2` optional behind the default `mmap` feature and disables that feature in Miri CI and the wasm dependency graph. - Uses structural `cfg(miri)` branches for allocator and compression paths Miri cannot execute. - Re-enables three persistence compaction tests after measuring them successfully under Miri. - Keeps normal native behavior unchanged and documents measured Miri exclusions at the affected tests. Verification included focused normal and Miri tests for persistence, rcstr, allocation accounting, compression, and the refactored leak-free predicate cases. <!-- NEXT_JS_LLM --> <!-- fleet fb42942f-173a-484c-b4de-4e18354834c6 --> --------- Co-authored-by: vercel-fleet-prod[bot] <318278635+vercel-fleet-prod[bot]@users.noreply.github.com> Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
If upgrade handles the Node version check, upgrade from Next version that allowed Node v18 -> Next version that require Node v20 can fail the upgrade. Next handles compatible Node version so proceed upgrade and let agent handle it afterwards.
…dis, and cross-page tag invalidation (#98716) Three runtime problems in `examples/cache-handler-redis`. ### 1. One new Redis connection per request, never closed Next.js constructs the singular `cacheHandler` class once per request (`new CurCacheHandler(...)` in `IncrementalCache`, which `route-module.ts` creates per request). The example's constructor calls `createClient()` + `connect()`, so every request opens a connection that is never closed. ``` $ redis-cli client list | wc -l # before 102 $ for i in $(seq 1 10); do curl -s -o /dev/null localhost:3000/cet; done $ redis-cli client list | wc -l 132 # +3 per request ``` With Redis' default `maxclients 10000`, a single instance runs out after a few thousand requests. ### 2. Requests hang while Redis is unavailable The README says the handlers "degrade gracefully when Redis is unavailable, so the app still builds and runs, just without a shared cache". At runtime they don't: with Redis stopped, every request on every instance blocks until Redis comes back. ``` $ docker stop cache-handler-redis $ curl -s -o /dev/null -m 60 -w "%{http_code} %{time_total}s\n" localhost:3000/cet 000 60.007305s # (uncapped: 188s, returned the moment Redis was started again) ``` Same when the app is started while Redis is down. Cause: node-redis keeps retrying in the background and `client.connect()` does not settle until a connection succeeds. Isolated: ```js const c = createClient({ url: "redis://localhost:6379" }); c.on("error", () => {}); await Promise.race([c.connect(), new Promise(r => setTimeout(() => r("pending"), 15000))]); // -> "pending" after 15s, isOpen=true isReady=false (redis@6.2.1) ``` Because each request built a new handler, each request awaited a fresh, never-settling `connect()` in `getClient()`. `remote-cache-handler.js` has one module-level client, but its `getClient()` awaits the same promise, so it hangs the same way if the app starts while Redis is down. ### 3. `updateTag` never reaches other pages' remote entries The remote handler's `get` never checks the entry's own tags. Next.js only passes soft tags to `getExpiration`, and the `"use cache"` wrapper only knows about tags revalidated in the current request, so `updateTag("time-data")` from `/cet` left the `/gmt` entry stale on every instance, including the one that ran it, until it expired (`cacheLife` `expire: 3600`). The [`cacheHandlers` docs](https://nextjs.org/docs/app/api-reference/config/next-config-js/cacheHandlers#get) say `get` should report an entry whose tag was invalidated as missing or stale. ### Fix - Hoist the client in `cache-handler.js` to module scope (the pattern `remote-cache-handler.js` already uses). - `getClient()` awaits the connect promise raced against a 1s `unref()`'d timer (suggested in review), then returns the client when `isReady` and `null` otherwise. Requests during startup still wait for the connection, and a down Redis costs one bounded wait instead of blocking every request. The client keeps retrying and `isReady` flips back on its own. - `disableOfflineQueue: true`, so a command issued while the connection is down rejects immediately (`ClientOfflineError`) instead of being queued for the 5s command timeout. - Entry `get` / `set` catch the Redis call only and degrade to a miss. - Remote `get` compares the entry's tags against their revalidation timestamps (one `MGET`) and misses when any is newer, the same comparison as the built-in handler. `getExpiration` returns `Date.now()` when Redis can't answer, so the entry is discarded rather than served. - `revalidateTag` / `updateTags` throw when Redis isn't ready, so an invalidation that never reached Redis surfaces as an error instead of a success whose entries come back once Redis does. ### After Two `next start` instances on one Redis, same script on `next@16.3.5` and `16.4.0-canary.35` (identical results), canary's handlers vs this PR: | scenario | before | after | | --- | --- | --- | | `updateTag` from `/cet`, read `/gmt` on both instances | stale | refreshed | | `revalidateTag("time-data", "max")` | neither page refreshed | both refreshed | | tag lookup fails | 500 | 200, regenerated | | request while Redis is stopped | no response | 200, uncached | | `updateTag` while Redis is stopped | no response | 500, error logged | | app started without Redis | no response | first ISR read waits ≤1.1s once | | Redis connections, 200 requests | 173 → 773 | 7 → 7 | | `/cet` under load (autocannon, 10 connections × 10s, `16.3.5`) | 332 req/s, hits Redis `maxclients` | 1,106 req/s median, p99 20ms, 3 connections | | 10 readers during 20 `updateTag`s: reads served a value from before a completed invalidation | 3,089 of 3,089 | 0 of 33,899 | | Redis down ~4s under load | timeouts, caching doesn't come back | 0 errors, caching resumes | The tag check costs one Redis round trip per remote hit (−9 to −13% on a route that only reads one remote entry). Handler-level cases, full tables and the benchmark breakdown are in [this comment](#98716 (comment)).
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )