Skip to content

Commit

Permalink
Let leakWarning carry the string representation, not the stack itself (
Browse files Browse the repository at this point in the history
…#22488)

At least with recent Chrome, I ran into a situation where an object of
mine apparently didn't get garbage collected because it was retained via
such a leakWarning stack trace. (I'm not sure about the internal
workings of that, though; but the object started to get garbage
collected once I did this change.)

I have no idea if there are any other up-/downsides to carrying the
stack vs. just the string here. And my explanation of what exactly it
fixed for me is probably a bit vague. But wanted to bring it up
nevertheless---maybe it is considered a good change anyway.
  • Loading branch information
stbergmann authored Oct 1, 2024
1 parent 214e851 commit 6cdebf1
Showing 1 changed file with 4 additions and 3 deletions.
7 changes: 4 additions & 3 deletions src/embind/embind.js
Original file line number Diff line number Diff line change
Expand Up @@ -1522,7 +1522,7 @@ var LibraryEmbind = {
// at run-time, not build-time.
finalizationRegistry = new FinalizationRegistry((info) => {
#if ASSERTIONS
console.warn(info.leakWarning.stack.replace(/^Error: /, ''));
console.warn(info.leakWarning);
#endif
releaseClassHandle(info.$$);
});
Expand All @@ -1538,13 +1538,14 @@ var LibraryEmbind = {
// This is more useful than the empty stacktrace of `FinalizationRegistry`
// callback.
var cls = $$.ptrType.registeredClass;
info.leakWarning = new Error(`Embind found a leaked C++ instance ${cls.name} <${ptrToString($$.ptr)}>.\n` +
var err = new Error(`Embind found a leaked C++ instance ${cls.name} <${ptrToString($$.ptr)}>.\n` +
"We'll free it automatically in this case, but this functionality is not reliable across various environments.\n" +
"Make sure to invoke .delete() manually once you're done with the instance instead.\n" +
"Originally allocated"); // `.stack` will add "at ..." after this sentence
if ('captureStackTrace' in Error) {
Error.captureStackTrace(info.leakWarning, RegisteredPointer_fromWireType);
Error.captureStackTrace(err, RegisteredPointer_fromWireType);
}
info.leakWarning = err.stack.replace(/^Error: /, '');
#endif
finalizationRegistry.register(handle, info, handle);
}
Expand Down

0 comments on commit 6cdebf1

Please sign in to comment.