Skip to content

Commit 8a2a490

Browse files
authored
fix(cloudflare): Use original waitUntil to not create a deadlock (#21197)
follow up to: #21156 This prevents a deadlock in the `waitUntil`, which happened in the sentry-mcp: <img width="972" height="119" alt="Screenshot 2026-05-27 at 17 04 19" src="https://github.com/user-attachments/assets/18ec0f64-c766-4f6f-82a6-9453280668a5" />
1 parent f7b506d commit 8a2a490

3 files changed

Lines changed: 106 additions & 4 deletions

File tree

packages/cloudflare/src/flush.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ type FlushLock = {
99

1010
type FlushLockRegistry = {
1111
readonly locks: Set<FlushLockInternal>;
12+
readonly originalWaitUntil: ExecutionContext['waitUntil'];
1213
};
1314

1415
type FlushLockInternal = FlushLock & {
@@ -18,6 +19,27 @@ type FlushLockInternal = FlushLock & {
1819

1920
const flushLockRegistries = new WeakMap<ExecutionContext['waitUntil'], FlushLockRegistry>();
2021

22+
/**
23+
* Returns the original (un-instrumented) waitUntil function for a context.
24+
* This should be used when calling waitUntil with flushAndDispose to avoid deadlock.
25+
*
26+
* The flush lock mechanism wraps context.waitUntil to track pending tasks.
27+
* If we call waitUntil(flushAndDispose(client)) through the instrumented version,
28+
* it creates a deadlock because:
29+
* 1. The instrumented waitUntil acquires the flush lock
30+
* 2. flushAndDispose calls client.flush() which waits for the lock to be released
31+
* 3. The lock won't be released until the waitUntil promise completes
32+
* 4. The waitUntil promise won't complete until flush() returns
33+
*
34+
* By using the original waitUntil for flush operations, we bypass this issue.
35+
*/
36+
export function getOriginalWaitUntil(context: ExecutionContext): ExecutionContext['waitUntil'] | undefined {
37+
// eslint-disable-next-line @typescript-eslint/unbound-method
38+
const currentWaitUntil = context.waitUntil;
39+
const original = flushLockRegistries.get(currentWaitUntil)?.originalWaitUntil;
40+
return original ?? currentWaitUntil;
41+
}
42+
2143
/**
2244
* Enhances the given execution context by wrapping its `waitUntil` method with a proxy
2345
* to monitor pending tasks, and provides a flusher function to ensure all tasks
@@ -67,8 +89,8 @@ function getOrCreateFlushLockRegistry(context: ExecutionContext): FlushLockRegis
6789
return existingRegistry;
6890
}
6991

70-
const registry: FlushLockRegistry = { locks: new Set() };
7192
const originalWaitUntil = context.waitUntil.bind(context) as typeof context.waitUntil;
93+
const registry: FlushLockRegistry = { locks: new Set(), originalWaitUntil };
7294
const instrumentedWaitUntil: typeof context.waitUntil = promise => {
7395
// Snapshot active locks so locks created after this call do not wait for earlier waitUntil work.
7496
const locks = [...registry.locks];

packages/cloudflare/src/request.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {
1414
} from '@sentry/core';
1515
import { captureIncomingRequestBody } from './integrations/httpServer';
1616
import type { CloudflareOptions } from './client';
17-
import { flushAndDispose } from './flush';
17+
import { flushAndDispose, getOriginalWaitUntil } from './flush';
1818
import { addCloudResourceContext, addCultureContext, addRequest } from './scope-utils';
1919
import { init } from './sdk';
2020
import { classifyResponseStreaming } from './utils/streaming';
@@ -47,7 +47,12 @@ export function wrapRequestHandler(
4747
const { options, request, captureErrors = true } = wrapperOptions;
4848
const context = wrapperOptions.context;
4949

50-
const waitUntil = context?.waitUntil?.bind?.(context);
50+
// Use getOriginalWaitUntil to get the un-instrumented waitUntil function.
51+
// This is crucial to avoid deadlock: the flush lock mechanism wraps waitUntil
52+
// to track pending tasks. If we use the instrumented version for flushAndDispose,
53+
// it acquires the lock, then flushAndDispose tries to wait for the same lock,
54+
// creating a deadlock.
55+
const waitUntil = context ? getOriginalWaitUntil(context)?.bind(context) : undefined;
5156

5257
const client = init({ ...options, ctx: context });
5358
isolationScope.setClient(client);

packages/cloudflare/test/flush.test.ts

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { type ExecutionContext } from '@cloudflare/workers-types';
22
import * as sentryCore from '@sentry/core';
33
import { type Client } from '@sentry/core';
44
import { describe, expect, it, onTestFinished, vi } from 'vitest';
5-
import { flushAndDispose, makeFlushLock } from '../src/flush';
5+
import { flushAndDispose, getOriginalWaitUntil, makeFlushLock } from '../src/flush';
66

77
describe('Flush buffer test', () => {
88
const waitUntilPromises: Promise<void>[] = [];
@@ -109,3 +109,78 @@ describe('flushAndDispose', () => {
109109
flushSpy.mockRestore();
110110
});
111111
});
112+
113+
describe('getOriginalWaitUntil', () => {
114+
it('returns the original waitUntil before instrumentation', () => {
115+
const originalWaitUntil = vi.fn();
116+
const context: ExecutionContext = {
117+
waitUntil: originalWaitUntil,
118+
passThroughOnException: vi.fn(),
119+
};
120+
121+
const result = getOriginalWaitUntil(context);
122+
expect(result).toBe(originalWaitUntil);
123+
});
124+
125+
it('returns the original waitUntil after instrumentation', () => {
126+
const originalWaitUntil = vi.fn();
127+
const context: ExecutionContext = {
128+
waitUntil: originalWaitUntil,
129+
passThroughOnException: vi.fn(),
130+
};
131+
132+
makeFlushLock(context);
133+
134+
const result = getOriginalWaitUntil(context);
135+
136+
expect(result).not.toBe(context.waitUntil);
137+
expect(result).toBeDefined();
138+
result!(Promise.resolve());
139+
expect(originalWaitUntil).toHaveBeenCalled();
140+
});
141+
142+
it('returns the original waitUntil after multiple instrumentations', () => {
143+
const originalWaitUntil = vi.fn();
144+
const context: ExecutionContext = {
145+
waitUntil: originalWaitUntil,
146+
passThroughOnException: vi.fn(),
147+
};
148+
149+
makeFlushLock(context);
150+
makeFlushLock(context);
151+
makeFlushLock(context);
152+
153+
const result = getOriginalWaitUntil(context);
154+
155+
expect(result).not.toBe(context.waitUntil);
156+
result!(Promise.resolve());
157+
expect(originalWaitUntil).toHaveBeenCalled();
158+
});
159+
160+
it('allows flushAndDispose to complete when called via original waitUntil', async () => {
161+
const waitUntilPromises: Promise<void>[] = [];
162+
const context: ExecutionContext = {
163+
waitUntil: vi.fn(promise => {
164+
waitUntilPromises.push(promise);
165+
}),
166+
passThroughOnException: vi.fn(),
167+
};
168+
169+
const lock = makeFlushLock(context);
170+
171+
const mockClient = {
172+
flush: vi.fn(async () => {
173+
await lock.finalize();
174+
return true;
175+
}),
176+
dispose: vi.fn(),
177+
} as unknown as Client;
178+
179+
const originalWaitUntil = getOriginalWaitUntil(context);
180+
originalWaitUntil!.call(context, flushAndDispose(mockClient));
181+
182+
await vi.waitFor(() => Promise.all(waitUntilPromises));
183+
expect(mockClient.flush).toHaveBeenCalled();
184+
expect(mockClient.dispose).toHaveBeenCalled();
185+
});
186+
});

0 commit comments

Comments
 (0)