fix: unset context on stale promises

When a stale promise is rejected in `async_derived`, and the promise eventually resolves, `d.resolve` will be noop and `d.promise.then(handler, ...)` will never run. That in turns means any restored context (via `(await save(..))()`) will never be unset. We have to handle this case and unset the context to prevent errors such as false-positive state mutation errors

Author: dummdidumm

.changeset/major-beans-fry.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: unset context on stale promises

packages/svelte/src/internal/client/reactivity/deriveds.js

@@ -113,20 +113,30 @@ export function async_derived(fn, location) {
 	// only suspend in async deriveds created on initialisation
 	var should_suspend = !active_reaction;
 
-	/** @type {Map<Batch, ReturnType<typeof deferred<V>>>} */
+	/** @type {Map<Batch, ReturnType<typeof deferred<V>> & { rejected?: boolean }>} */
 	var deferreds = new Map();
 
 	async_effect(() => {
 		if (DEV) current_async_effect = active_effect;
 
-		/** @type {ReturnType<typeof deferred<V>>} */
+		/** @type {ReturnType<typeof deferred<V>> & { rejected?: boolean }} */
 		var d = deferred();
 		promise = d.promise;
 
 		try {
 			// If this code is changed at some point, make sure to still access the then property
 			// of fn() to read any signals it might access, so that we track them as dependencies.
-			Promise.resolve(fn()).then(d.resolve, d.reject);
+			Promise.resolve(fn()).then((v) => {
+				if (d.rejected) {
+					// If we rejected this stale promise, d.resolve
+					// is a noop (d.promise.then(handler) below will never run).
+					// In this case we need to unset the restored context here
+					// to avoid leaking it (and e.g. cause false-positive mutation errors).
+					unset_context();
+				} else {
+					d.resolve(v);
+				}
+			}, d.reject);
 		} catch (error) {
 			d.reject(error);
 		}
@@ -141,7 +151,11 @@ export function async_derived(fn, location) {
 			if (!pending) {
 				batch.increment();
 
-				deferreds.get(batch)?.reject(STALE_REACTION);
+				var previous_deferred = deferreds.get(batch);
+				if (previous_deferred) {
+					previous_deferred.rejected = true;
+					previous_deferred.reject(STALE_REACTION);
+				}
 				deferreds.set(batch, d);
 			}
 		}

packages/svelte/tests/runtime-runes/samples/async-resolve-stale/_config.js

@@ -0,0 +1,26 @@
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, target }) {
+		// We gotta wait a bit more in this test because of the macrotasks in App.svelte
+		function macrotask(t = 3) {
+			return new Promise((r) => setTimeout(r, t));
+		}
+
+		await macrotask();
+		assert.htmlEqual(target.innerHTML, '<input> 1 | ');
+
+		const [input] = target.querySelectorAll('input');
+
+		input.value = '1';
+		input.dispatchEvent(new Event('input', { bubbles: true }));
+		await macrotask();
+		assert.htmlEqual(target.innerHTML, '<input> 1 | ');
+
+		input.value = '12';
+		input.dispatchEvent(new Event('input', { bubbles: true }));
+		await macrotask(6);
+		// TODO this is wrong (separate bug), this should be 3 | 12
+		assert.htmlEqual(target.innerHTML, '<input> 5 | 12');
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-resolve-stale/main.svelte

@@ -0,0 +1,38 @@
+<script>
+	let count = $state(0);
+	let value = $state('');
+	let prev;
+
+	function asd(v) {
+		const r = Promise.withResolvers();
+
+		if (prev || v === '') {
+			console.log('hello', !!prev)
+			Promise.resolve().then(async () => {
+				console.log('count++')
+				count++;
+				r.resolve(v);
+				await new Promise(r => setTimeout(r, 0));
+				// TODO with a microtask like below it still throws a mutation error
+				// await Promise.resolve();
+				prev?.resolve();
+			})
+		} else {
+			console.log('other')
+			prev = Promise.withResolvers();
+			prev.promise.then(() => {
+				console.log('other coun++')
+				count++;
+				r.resolve(v)
+			})
+		}
+		
+		return r.promise;
+	}
+
+	const x = $derived(await asd(value))
+</script>
+
+<input bind:value />
+
+{count} | {x}