Skip to content
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.

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

fix memory leak in ERR_INVALID_ARG_TYPE #17169

Merged
merged 5 commits into from
Feb 11, 2025
Merged

Conversation

nektro
Copy link
Member

@nektro nektro commented Feb 8, 2025

dont use ZigString like that on Zig<->Cpp boundary

verified memory leak regression fix with

import dns from "node:dns";
while (true) {
  try {
    dns.resolve(242);
  } catch (e) {
    e.message;
  }
}

dont use ZigString like that on Zig<->Cpp boundary
@robobun
Copy link

robobun commented Feb 8, 2025

Updated 6:49 PM PT - Feb 10th, 2025

@nektro, your commit 83533d4 has 6 failures in Build #11407:


🧪   try this PR locally:

bunx bun-pr 17169

@nektro nektro marked this pull request as ready for review February 8, 2025 09:07
@nektro
Copy link
Member Author

nektro commented Feb 8, 2025

this leaks much more consistently but still takes some time to rise, will add a test that stresses it

import dns from "node:dns";
while (true) {
  try {
    dns.resolve({ __proto__: null });
  } catch (e) {
    e.message;
  }
}

@nektro nektro requested a review from Jarred-Sumner February 11, 2025 02:49
@Jarred-Sumner Jarred-Sumner merged commit 7adb2b9 into main Feb 11, 2025
50 of 69 checks passed
@Jarred-Sumner Jarred-Sumner deleted the nektro-patch-29476 branch February 11, 2025 02:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants