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 leaks in test/correctness/memoize.cpp #7705

Merged
merged 17 commits into from
Aug 5, 2023

Conversation

abadams
Copy link
Member

@abadams abadams commented Jul 24, 2023

This was a little tricky, because it turns out any custom allocator used with memoized allocations needs to be compatible with the vanilla halide_free, because that's what's going to get called when the runtime shuts down.

Edit: The above issue is now resolved by just digging out the malloc/free from the runtime and wrapping them. Making a compatible one just didn't work on windows, as the malloc that gets called from the test seems to be different to the malloc that is resolved by the JIT-compiled runtime.

Built on #7700

@steven-johnson steven-johnson changed the base branch from main to abadams/remove_parameter_self_references July 25, 2023 00:16
@steven-johnson steven-johnson changed the base branch from abadams/remove_parameter_self_references to main July 25, 2023 00:16
@steven-johnson steven-johnson self-requested a review July 25, 2023 00:16
@steven-johnson
Copy link
Contributor

Looks like an error on correctness_memoize on Windows... https://buildbot.halide-lang.org/master/#/builders/148/builds/60

@abadams abadams merged commit c254043 into main Aug 5, 2023
3 checks passed
ardier pushed a commit to ardier/Halide-mutation that referenced this pull request Mar 3, 2024
* Fix leaks caused by self-referential parameter constraints

* Add comment

* Add missing overrides

* Fix reported leaks in memoize test

by explicitly releasing the shared runtime at the end of the test

* Use const refs for non-mutated args

* Hopefully fix for windows

* Fix for 32-bit pointers

* Don't use _aligned_malloc

It requires _aligned_free, which the runtime aint gonna do

* Fix other memoize test

* Use runtime built-in malloc/free

On windows mixing and matching mallocs and frees doesn't work well.

* Fix comment

---------

Co-authored-by: Steven Johnson <srj@google.com>
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