Skip to content

Conversation

@awelzel
Copy link
Collaborator

@awelzel awelzel commented Nov 19, 2025

No description provided.

When running with ASAN, StringVals continued to be referenced at shutdown
because V8 wouldn't run GC and some string resources would continue to
exists holding references. This is unfortunate for testing.

Rework Done() to force garbage collection using --expose-gc. Also call
some global shutdown functions for V8 Node.js and libuv.

Modify some of the tests to disable leak checking, or call terminate()
instead of exit. The issue here is that calling process.exit() doesn't
go through Plugin::Done() and so the cleanup does not run. This could
probably be improved with a custom exit handler and splitting Done()
into a helper for cleaning up, but that's for another time.

With these changes, it's now possible to remove the suppression
configuration altogether, so I think that was worth it.
That one used the old analyzer_confirmation() event. Switch to
analyzer_confirmation_info().
@awelzel
Copy link
Collaborator Author

awelzel commented Nov 19, 2025

Grumble, Ubuntu 24.04 with the distro provided libnode-dev package hangs on the zeekjs/js/finalization-registry.sh test. Will need some staring.

This seems to fix the FinalizationRegistry test (and make it work).
@awelzel awelzel force-pushed the topic/awelzel/fix-asan-shutdown-leaks branch from 6b496bf to 7a1a97f Compare November 19, 2025 20:06
Copy link
Collaborator

@timwoj timwoj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I'm kind of surprised that we weren't already calling the garbage collector at shutdown.

@timwoj
Copy link
Collaborator

timwoj commented Nov 19, 2025

I can also confirm that this fixes the Zeek javascript tests if you run an asan build with detect_leaks=1 turned on, which was the entire impetus for looking at this in the first place. See zeek/zeek#4423, which should be closed once this is moved into Zeek master.

@awelzel awelzel merged commit 6f94df1 into main Nov 20, 2025
7 checks passed
@awelzel awelzel deleted the topic/awelzel/fix-asan-shutdown-leaks branch November 20, 2025 09:17
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.

2 participants