Skip to content

Commit 7ae752e

Browse files
carterworksclaude
carterworks
and
claude
authored
Fix memory leaks causing OOM in 1GB container (#44)
* Fix memory leaks causing OOM in 1GB container - Close JSDOM windows after use in clipper.ts and youtubeExtractor.ts to release DOM trees, event listeners, and window objects - Remove memory-heavy JSDOM options (resources: "usable", pretendToBeVisual, includeNodeLocations, storageQuota) that were loading external resources - Lazy-load DOMPurify instance in summarizer.ts to avoid holding JSDOM in memory when not needed - Add Bun memory optimization flags to Dockerfile: - BUN_JSC_forceGCSlowPaths=1 for more aggressive garbage collection - --smol flag for reduced memory footprint Each JSDOM instance holds 5-50MB+ depending on page size. Without explicit cleanup via window.close(), these accumulate and cause OOM kills. * Add memory verification script for JSDOM cleanup testing Includes tests for: - JSDOM memory behavior with/without window.close() - Impact of 'resources: usable' option - Memory stability over repeated iterations Documents references to JSDOM issues that confirm the fix approach. * Add package-lock.json from npm install * Fix Bun test API usage - import from bun:test * Remove package-lock.json --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 1943a11 commit 7ae752e

File tree

6 files changed

+416
-113
lines changed

6 files changed

+416
-113
lines changed

‎Dockerfile‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,5 +43,12 @@ COPY --from=build /app/package.json /app/package.json
4343
# Start the server by default, this can be overwritten at runtime
4444
ENV HOST=${HOST}
4545
ENV PORT=${PORT}
46+
47+
# Memory management settings for Bun runtime
48+
# BUN_JSC_forceGCSlowPaths=1 makes garbage collection more aggressive
49+
# This helps prevent OOM in memory-constrained containers
50+
ENV BUN_JSC_forceGCSlowPaths=1
51+
4652
EXPOSE ${PORT}
47-
CMD [ "bun", "run", "dist/server/entry.mjs" ]
53+
# Use --smol flag for reduced memory footprint
54+
CMD [ "bun", "--smol", "run", "dist/server/entry.mjs" ]

‎scripts/verify-memory-fixes.ts‎

Lines changed: 272 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,272 @@
1+
/**
2+
* Memory verification script for JSDOM cleanup fixes
3+
*
4+
* Run with: bun run scripts/verify-memory-fixes.ts
5+
*
6+
* This script verifies that:
7+
* 1. JSDOM window.close() actually frees memory
8+
* 2. The clipper function doesn't leak memory across multiple calls
9+
* 3. Memory stays stable under repeated scraping
10+
*
11+
* VERIFICATION SUMMARY (from research):
12+
* - window.close() is the primary cleanup method for JSDOM
13+
* - Asynchronous cleanup (after work is done) is more effective than immediate close
14+
* - 'resources: "usable"' causes significant memory retention due to external fetches
15+
* - The fixes in clipper.ts and youtubeExtractor.ts properly wrap JSDOM usage
16+
* in try/finally blocks, ensuring cleanup even on errors
17+
*
18+
* References:
19+
* - https://github.com/jsdom/jsdom/issues/1682 (window.close() timing)
20+
* - https://github.com/jsdom/jsdom/issues/1493 (resources: usable leaks)
21+
*/
22+
23+
import { JSDOM, VirtualConsole } from "jsdom";
24+
25+
function formatBytes(bytes: number): string {
26+
const mb = bytes / 1024 / 1024;
27+
return `${mb.toFixed(2)} MB`;
28+
}
29+
30+
function getMemoryUsage(): { heapUsed: number; heapTotal: number; rss: number } {
31+
if (typeof Bun !== "undefined") {
32+
// Bun environment
33+
const usage = process.memoryUsage();
34+
return {
35+
heapUsed: usage.heapUsed,
36+
heapTotal: usage.heapTotal,
37+
rss: usage.rss,
38+
};
39+
}
40+
// Node environment
41+
return process.memoryUsage();
42+
}
43+
44+
function forceGC(): void {
45+
if (typeof Bun !== "undefined") {
46+
// Bun's garbage collection
47+
Bun.gc(true);
48+
} else if (global.gc) {
49+
// Node with --expose-gc flag
50+
global.gc();
51+
}
52+
}
53+
54+
async function testJSDOMWithoutCleanup(iterations: number): Promise<void> {
55+
console.log(`\n--- Test: JSDOM WITHOUT cleanup (${iterations} iterations) ---`);
56+
57+
const initialMemory = getMemoryUsage();
58+
console.log(`Initial heap: ${formatBytes(initialMemory.heapUsed)}`);
59+
60+
const htmlContent = `
61+
<!DOCTYPE html>
62+
<html>
63+
<head><title>Test Page</title></head>
64+
<body>
65+
<article>
66+
<h1>Test Article</h1>
67+
<p>${"Lorem ipsum dolor sit amet. ".repeat(1000)}</p>
68+
<img src="test.jpg" data-src="lazy.jpg">
69+
<pre><code>console.log("test");</code></pre>
70+
</article>
71+
</body>
72+
</html>
73+
`;
74+
75+
// Simulate NOT closing JSDOM (the old behavior)
76+
const leakedInstances: JSDOM[] = [];
77+
for (let i = 0; i < iterations; i++) {
78+
const dom = new JSDOM(htmlContent, {
79+
virtualConsole: new VirtualConsole(),
80+
});
81+
// Simulate some DOM operations
82+
dom.window.document.querySelectorAll("p");
83+
dom.window.document.querySelectorAll("img");
84+
85+
// NOT calling dom.window.close() - this is the leak!
86+
leakedInstances.push(dom); // Keep reference to prevent GC
87+
}
88+
89+
forceGC();
90+
await new Promise(r => setTimeout(r, 100));
91+
92+
const afterMemory = getMemoryUsage();
93+
console.log(`After heap: ${formatBytes(afterMemory.heapUsed)}`);
94+
console.log(`Memory increase: ${formatBytes(afterMemory.heapUsed - initialMemory.heapUsed)}`);
95+
console.log(`Per iteration: ${formatBytes((afterMemory.heapUsed - initialMemory.heapUsed) / iterations)}`);
96+
97+
// Cleanup for next test
98+
for (const dom of leakedInstances) {
99+
dom.window.close();
100+
}
101+
forceGC();
102+
}
103+
104+
async function testJSDOMWithCleanup(iterations: number): Promise<void> {
105+
console.log(`\n--- Test: JSDOM WITH cleanup (${iterations} iterations) ---`);
106+
107+
forceGC();
108+
await new Promise(r => setTimeout(r, 100));
109+
110+
const initialMemory = getMemoryUsage();
111+
console.log(`Initial heap: ${formatBytes(initialMemory.heapUsed)}`);
112+
113+
const htmlContent = `
114+
<!DOCTYPE html>
115+
<html>
116+
<head><title>Test Page</title></head>
117+
<body>
118+
<article>
119+
<h1>Test Article</h1>
120+
<p>${"Lorem ipsum dolor sit amet. ".repeat(1000)}</p>
121+
<img src="test.jpg" data-src="lazy.jpg">
122+
<pre><code>console.log("test");</code></pre>
123+
</article>
124+
</body>
125+
</html>
126+
`;
127+
128+
// Simulate proper cleanup (the new behavior)
129+
for (let i = 0; i < iterations; i++) {
130+
const dom = new JSDOM(htmlContent, {
131+
virtualConsole: new VirtualConsole(),
132+
});
133+
try {
134+
// Simulate some DOM operations
135+
dom.window.document.querySelectorAll("p");
136+
dom.window.document.querySelectorAll("img");
137+
} finally {
138+
// Proper cleanup!
139+
dom.window.close();
140+
}
141+
}
142+
143+
forceGC();
144+
await new Promise(r => setTimeout(r, 100));
145+
146+
const afterMemory = getMemoryUsage();
147+
console.log(`After heap: ${formatBytes(afterMemory.heapUsed)}`);
148+
console.log(`Memory increase: ${formatBytes(afterMemory.heapUsed - initialMemory.heapUsed)}`);
149+
console.log(`Per iteration: ${formatBytes((afterMemory.heapUsed - initialMemory.heapUsed) / iterations)}`);
150+
}
151+
152+
async function testResourcesUsableOption(): Promise<void> {
153+
console.log(`\n--- Test: JSDOM 'resources: usable' memory impact ---`);
154+
155+
forceGC();
156+
await new Promise(r => setTimeout(r, 100));
157+
158+
const htmlContent = `
159+
<!DOCTYPE html>
160+
<html>
161+
<head><title>Test Page</title></head>
162+
<body>
163+
<article>
164+
<h1>Test Article</h1>
165+
<p>${"Lorem ipsum dolor sit amet. ".repeat(500)}</p>
166+
</article>
167+
</body>
168+
</html>
169+
`;
170+
171+
// Test WITHOUT resources: usable (new behavior)
172+
const mem1 = getMemoryUsage();
173+
const dom1 = new JSDOM(htmlContent, {
174+
virtualConsole: new VirtualConsole(),
175+
});
176+
const mem2 = getMemoryUsage();
177+
console.log(`Without 'resources: usable': ${formatBytes(mem2.heapUsed - mem1.heapUsed)}`);
178+
dom1.window.close();
179+
180+
forceGC();
181+
await new Promise(r => setTimeout(r, 100));
182+
183+
// Test WITH resources: usable (old behavior)
184+
const mem3 = getMemoryUsage();
185+
const dom2 = new JSDOM(htmlContent, {
186+
virtualConsole: new VirtualConsole(),
187+
resources: "usable",
188+
pretendToBeVisual: true,
189+
});
190+
const mem4 = getMemoryUsage();
191+
console.log(`With 'resources: usable': ${formatBytes(mem4.heapUsed - mem3.heapUsed)}`);
192+
dom2.window.close();
193+
}
194+
195+
async function runMemoryStabilityTest(iterations: number): Promise<void> {
196+
console.log(`\n--- Test: Memory stability over ${iterations} iterations ---`);
197+
198+
forceGC();
199+
await new Promise(r => setTimeout(r, 100));
200+
201+
const htmlContent = `
202+
<!DOCTYPE html>
203+
<html>
204+
<head><title>Test Page</title></head>
205+
<body>
206+
<article>
207+
<h1>Test Article</h1>
208+
<p>${"Lorem ipsum dolor sit amet. ".repeat(1000)}</p>
209+
</article>
210+
</body>
211+
</html>
212+
`;
213+
214+
const memorySnapshots: number[] = [];
215+
216+
for (let i = 0; i < iterations; i++) {
217+
const dom = new JSDOM(htmlContent, {
218+
virtualConsole: new VirtualConsole(),
219+
});
220+
try {
221+
dom.window.document.querySelectorAll("p");
222+
} finally {
223+
dom.window.close();
224+
}
225+
226+
if (i % 10 === 0) {
227+
forceGC();
228+
await new Promise(r => setTimeout(r, 10));
229+
memorySnapshots.push(getMemoryUsage().heapUsed);
230+
}
231+
}
232+
233+
console.log("Memory over time (every 10 iterations):");
234+
memorySnapshots.forEach((mem, idx) => {
235+
console.log(` Iteration ${idx * 10}: ${formatBytes(mem)}`);
236+
});
237+
238+
const firstHalf = memorySnapshots.slice(0, memorySnapshots.length / 2);
239+
const secondHalf = memorySnapshots.slice(memorySnapshots.length / 2);
240+
const avgFirst = firstHalf.reduce((a, b) => a + b, 0) / firstHalf.length;
241+
const avgSecond = secondHalf.reduce((a, b) => a + b, 0) / secondHalf.length;
242+
243+
const drift = avgSecond - avgFirst;
244+
console.log(`\nMemory drift (2nd half avg - 1st half avg): ${formatBytes(drift)}`);
245+
246+
if (drift > 5 * 1024 * 1024) { // 5MB threshold
247+
console.log("⚠️ WARNING: Significant memory drift detected - possible leak!");
248+
} else {
249+
console.log("✅ Memory appears stable");
250+
}
251+
}
252+
253+
async function main(): Promise<void> {
254+
console.log("=== JSDOM Memory Verification Script ===");
255+
console.log(`Runtime: ${typeof Bun !== "undefined" ? "Bun" : "Node.js"}`);
256+
console.log(`Initial RSS: ${formatBytes(getMemoryUsage().rss)}`);
257+
258+
const iterations = 50;
259+
260+
await testJSDOMWithoutCleanup(iterations);
261+
await testJSDOMWithCleanup(iterations);
262+
await testResourcesUsableOption();
263+
await runMemoryStabilityTest(100);
264+
265+
console.log("\n=== Summary ===");
266+
console.log("If 'WITH cleanup' shows significantly less memory increase than");
267+
console.log("'WITHOUT cleanup', the window.close() fix is working correctly.");
268+
console.log("\nIf memory drift is minimal in the stability test, the fixes");
269+
console.log("should prevent OOM in production.");
270+
}
271+
272+
main().catch(console.error);

‎src/pages/api/article-count.test.ts‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,24 @@
1+
import { beforeAll, describe, expect, it } from "bun:test";
12
import { cache } from "../../services/cache";
23

3-
Bun.test.describe("Article Count API", () => {
4-
Bun.test.beforeAll(async () => {
4+
describe("Article Count API", () => {
5+
beforeAll(async () => {
56
// Ensure we have a clean state for testing
67
});
78

8-
Bun.test.it("should return article count", async () => {
9+
it("should return article count", async () => {
910
const count = await cache.getArticleCount();
10-
Bun.test.expect(typeof count).toBe("number");
11-
Bun.test.expect(count).toBeGreaterThanOrEqual(0);
11+
expect(typeof count).toBe("number");
12+
expect(count).toBeGreaterThanOrEqual(0);
1213
});
1314

14-
Bun.test.it("should return count as string in API", async () => {
15+
it("should return count as string in API", async () => {
1516
const { GET } = await import("./article-count");
1617
const mockContext = {
1718
request: new Request("http://localhost/api/article-count"),
1819
} as Parameters<typeof GET>[0];
1920
const response = await GET(mockContext);
2021
const text = await response.text();
21-
Bun.test.expect(Number.parseInt(text, 10)).toBeGreaterThanOrEqual(0);
22+
expect(Number.parseInt(text, 10)).toBeGreaterThanOrEqual(0);
2223
});
2324
});

0 commit comments

Comments
 (0)