New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix memory leak #3887

Merged

Jarred-Sumner merged 3 commits into main from jarred/leak-fixes

Jul 30, 2023

+191 −87

Collaborator

Jarred-Sumner commented Jul 30, 2023

What does this PR do?

We weren't freeing strong references. Everything using JSC.Strong leaked memory. The code using .protect() or .unprotect() is unaffected.

Fixes #3065

Jarred-Sumner added 3 commits

July 29, 2023 21:49


          Fix memory leak

096c390


          Remove an extra copy

5222aac


          Further fixes

d17df08

Jarred-Sumner mentioned this pull request

Memory usage keeps increasing while heapSize remains unchanged #3065

Closed

Contributor

github-actions bot commented Jul 30, 2023

❌ @Jarred-Sumner 3 files with test failures on linux-x64:

test/js/node/child_process/child_process-node.test.js
test/js/node/tls/node-tls-connect.test.ts
test/js/node/watch/fs.watch.test.ts

View test output

^{#d9062e9156f1a22d5272bbd138d7c36b5b76cee9}

Contributor

github-actions bot commented Jul 30, 2023

❌ @Jarred-Sumner 2 files with test failures on linux-x64-baseline:

test/js/node/child_process/child_process-node.test.js
test/js/node/tls/node-tls-connect.test.ts

View test output

^{#d9062e9156f1a22d5272bbd138d7c36b5b76cee9}

Collaborator Author

Jarred-Sumner commented Jul 30, 2023

@cirospaciari can you update this test with a hardcoded local peer certificate to prevent it from being flaky going forward?

Jarred-Sumner merged commit 1db119e into main

Jarred-Sumner deleted the jarred/leak-fixes branch

July 30, 2023 09:03

Contributor

github-actions bot commented Jul 30, 2023

❌ @Jarred-Sumner 9 files with test failures on bun-darwin-aarch64:

test/js/bun/http/serve.test.ts
test/js/bun/spawn/spawn-streaming-stdout.test.ts
test/js/bun/spawn/spawn.test.ts
test/js/bun/test/test-test.test.ts
test/js/deno/crypto/webcrypto.test.ts
test/js/node/child_process/child_process-node.test.js
test/js/node/tls/node-tls-connect.test.ts
test/js/third_party/socket.io/socket.io-connection-state-recovery.test.ts
test/js/web/timers/setTimeout.test.js

View test output

^{#d9062e9156f1a22d5272bbd138d7c36b5b76cee9}

trnxdev pushed a commit to trnxdev/bun that referenced this pull request


          Fix memory leak (oven-sh#3887)

438da16

* Fix memory leak

* Remove an extra copy

* Further fixes

---------

Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet