Visitar URL original
fix: avoid Blob upload body to prevent native ArrayBuffer leak · gabrielbryk/pyroscope-nodejs@2cc438c · GitHub
Skip to content

Commit 2cc438c

Browse files
gabrielbrykgabrielbryk
committed
fix: avoid Blob upload body to prevent native ArrayBuffer leak
The profile uploader built the request body as a FormData containing a Blob. undici's fetch serializes Blob parts via Blob.prototype.stream(), which on Node 24.16.0-24.18.0 pins the source ArrayBuffer in an eternal handle (nodejs/node#63574). Each flush leaks one payload-sized ArrayBuffer, producing a steady ~45-80 MB/day native-memory leak in production for a long-lived process running two profilers. Serialize the multipart/form-data body by hand into a plain Uint8Array instead. The wire bytes are identical to what undici emits for the equivalent FormData/Blob (same part framing, headers, CRLF placement); only the arbitrary boundary token differs. A typed-array body never touches Blob.stream(), so the leak cannot occur on any Node version, and it also skips a wasteful Buffer -> Blob -> stream round-trip on every flush. Also always drain the response body (response.text()) on the success path, which previously left the 200-response body unconsumed. Refs: nodejs/node#63574, nodejs/node#63577, nodejs/node#64105
1 parent be4be7a commit 2cc438c

3 files changed

Lines changed: 154 additions & 12 deletions

File tree

‎src/pyroscope-api-exporter.ts‎

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@ import { URL } from 'node:url';
33
import { encode } from '@datadog/pprof';
44
import { Profile } from 'pprof-format';
55
import { ProfileExport, ProfileExporter } from './profile-exporter.js';
6+
import {
7+
buildProfileMultipartBody,
8+
MultipartRequestBody,
9+
} from './utils/build-profile-multipart-body.js';
610
import { dateToUnixTimestamp } from './utils/date-to-unix-timestamp.js';
711
import { processProfile } from './utils/process-profile.js';
812
import debug from 'debug';
@@ -93,9 +97,9 @@ export class PyroscopeApiExporter implements ProfileExporter {
9397
return arrayBuffer;
9498
}
9599

96-
private async buildUploadProfileFormData(
100+
private async buildUploadProfileRequestBody(
97101
profile: Profile
98-
): Promise<FormData> {
102+
): Promise<MultipartRequestBody> {
99103
const processedProfile: Profile = processProfile(profile, {
100104
stripFilenames: this.config.stripFilenames,
101105
shortenPaths: this.config.shortenPaths,
@@ -104,30 +108,34 @@ export class PyroscopeApiExporter implements ProfileExporter {
104108
const arrayBuffer: Uint8Array<ArrayBuffer> =
105109
this.buildArrayBuffer(profileBuffer);
106110

107-
const formData: FormData = new FormData();
108-
formData.append('profile', new Blob([arrayBuffer]), 'profile');
109-
110-
return formData;
111+
return buildProfileMultipartBody(arrayBuffer);
111112
}
112113

113114
private async uploadProfile(profileExport: ProfileExport): Promise<void> {
114-
const formData: FormData = await this.buildUploadProfileFormData(
115-
profileExport.profile
116-
);
115+
const { body, contentType }: MultipartRequestBody =
116+
await this.buildUploadProfileRequestBody(profileExport.profile);
117+
118+
const headers: Headers = this.buildRequestHeaders();
119+
headers.set('content-type', contentType);
117120

118121
try {
119122
const response = await fetch(
120123
this.buildEndpointUrl(profileExport).toString(),
121124
{
122-
body: formData,
123-
headers: this.buildRequestHeaders(),
125+
body,
126+
headers,
124127
method: 'POST',
125128
}
126129
);
127130

131+
// Always drain the response body. The success path (HTTP 200) previously
132+
// left the body unconsumed, which keeps the connection from being
133+
// released back to the pool promptly and retains the response buffer.
134+
const responseText: string = await response.text();
135+
128136
if (!response.ok) {
129137
log('Server rejected data ingest: HTTP %d', response.status);
130-
log(await response.text());
138+
log(responseText);
131139
}
132140
} catch (error: unknown) {
133141
if (error instanceof Error) {

‎src/utils/build-profile-multipart-body.ts‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import { randomBytes } from 'node:crypto';
2+
3+
const CRLF = '\r\n';
4+
5+
export interface MultipartRequestBody {
6+
body: Uint8Array<ArrayBuffer>;
7+
contentType: string;
8+
}
9+
10+
// Serializes a single `profile` part into a multipart/form-data request body
11+
// as a plain, fully-materialized Uint8Array instead of a `FormData` + `Blob`.
12+
//
13+
// WHY this is a hand-rolled buffer rather than `FormData`:
14+
//
15+
// When the request body is a `FormData` containing a `Blob`, undici's `fetch`
16+
// serializes each blob part by calling `blob.stream()`. On Node 24.16.0 -
17+
// 24.18.0 `Blob.prototype.stream()` pins the source `ArrayBuffer` in an
18+
// eternal (never-released) handle, so every flush leaks one payload-sized
19+
// ArrayBuffer. With two profilers flushing on the default interval this is a
20+
// steady, monotonic native-memory leak (~45-80 MB/day in production) that a
21+
// long-lived process never recovers.
22+
// - node core bug: https://github.com/nodejs/node/issues/63574
23+
// - core fix (main, not yet on 24.x): https://github.com/nodejs/node/pull/63577
24+
// - 24.x backport tracking: https://github.com/nodejs/node/issues/64105
25+
//
26+
// Passing a typed-array body sidesteps `Blob.stream()` entirely, so the leak
27+
// cannot occur on any Node version. It also avoids a wasteful
28+
// Buffer -> Blob -> stream round-trip on every flush on all versions. The bytes
29+
// produced here are wire-identical to what undici emits for the equivalent
30+
// `FormData`/`Blob` (same part framing, headers, and CRLF placement); only the
31+
// boundary token differs, which is arbitrary by design.
32+
export function buildProfileMultipartBody(
33+
content: Uint8Array<ArrayBuffer>
34+
): MultipartRequestBody {
35+
const boundary = `----pyroscope${randomBytes(16).toString('hex')}`;
36+
37+
const header: Buffer = Buffer.from(
38+
`--${boundary}${CRLF}` +
39+
`Content-Disposition: form-data; name="profile"; filename="profile"${CRLF}` +
40+
`Content-Type: application/octet-stream${CRLF}${CRLF}`,
41+
'utf8'
42+
);
43+
const footer: Buffer = Buffer.from(`${CRLF}--${boundary}--${CRLF}`, 'utf8');
44+
45+
const body: Buffer<ArrayBuffer> = Buffer.concat([header, content, footer]);
46+
47+
return {
48+
body: new Uint8Array(body.buffer, body.byteOffset, body.byteLength),
49+
contentType: `multipart/form-data; boundary=${boundary}`,
50+
};
51+
}

‎test/build-profile-multipart-body.test.ts‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
import { describe, it } from 'node:test';
2+
import { strict as assert } from 'node:assert';
3+
import { randomBytes } from 'node:crypto';
4+
import { Readable } from 'node:stream';
5+
6+
import busboy from 'busboy';
7+
8+
import { buildProfileMultipartBody } from '../src/utils/build-profile-multipart-body.js';
9+
10+
interface ParsedPart {
11+
name: string;
12+
filename: string | undefined;
13+
mimeType: string;
14+
data: Buffer;
15+
}
16+
17+
// Parse a multipart/form-data payload with busboy, the same parser the
18+
// profiler integration tests use to decode uploads.
19+
const parseMultipart = (
20+
body: Uint8Array,
21+
contentType: string
22+
): Promise<ParsedPart[]> =>
23+
new Promise<ParsedPart[]>((resolve, reject) => {
24+
const bb = busboy({ headers: { 'content-type': contentType } });
25+
const partPromises: Promise<ParsedPart>[] = [];
26+
27+
bb.on('file', (name, stream, info) => {
28+
partPromises.push(
29+
stream.toArray().then((chunks: Buffer[]) => ({
30+
name,
31+
filename: info.filename,
32+
mimeType: info.mimeType,
33+
data: Buffer.concat(chunks),
34+
}))
35+
);
36+
});
37+
// 'close' can fire before the per-file toArray() promises settle, so wait
38+
// on them before resolving.
39+
bb.on('close', () => {
40+
Promise.all(partPromises).then(resolve).catch(reject);
41+
});
42+
bb.on('error', reject);
43+
44+
Readable.from(Buffer.from(body)).pipe(bb);
45+
});
46+
47+
describe('buildProfileMultipartBody', () => {
48+
it('sets a multipart/form-data content type with a pyroscope boundary', () => {
49+
const { contentType } = buildProfileMultipartBody(
50+
new Uint8Array([1, 2, 3])
51+
);
52+
53+
assert.match(
54+
contentType,
55+
/^multipart\/form-data; boundary=----pyroscope[0-9a-f]{32}$/
56+
);
57+
});
58+
59+
it('serializes the profile part with bytes intact (byte-equal to input)', async () => {
60+
// Random bytes exercise binary content, including sequences that could
61+
// resemble CRLF/boundary framing.
62+
const content = new Uint8Array(randomBytes(4096).buffer);
63+
64+
const { body, contentType } = buildProfileMultipartBody(content);
65+
const parts = await parseMultipart(body, contentType);
66+
67+
assert.equal(parts.length, 1);
68+
const [part] = parts;
69+
assert.equal(part.name, 'profile');
70+
assert.equal(part.filename, 'profile');
71+
assert.equal(part.mimeType, 'application/octet-stream');
72+
// Byte-for-byte equality with the buffer handed to the serializer.
73+
assert.deepEqual(new Uint8Array(part.data), content);
74+
});
75+
76+
it('produces a body that is not a Blob (avoids Blob.stream leak)', () => {
77+
const { body } = buildProfileMultipartBody(
78+
new Uint8Array([0, 255, 10, 13])
79+
);
80+
81+
assert.ok(body instanceof Uint8Array);
82+
});
83+
});

0 commit comments

Comments
 (0)