Skip to content

aotcompile: destroy LLVM context after serializing combined module #59329

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

Merged
merged 1 commit into from
Aug 22, 2025
+8 −3
Merged

aotcompile: destroy LLVM context after serializing combined module #59329

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/aotcompile.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2271,9 +2271,14 @@ void jl_dump_native_impl(void *native_code,
auto lock = TSCtx.getLock();
auto dataM = data->M.getModuleUnlocked();

// Delete data when add_output thinks it's done with it
// Saves memory for use when multithreading
data_outputs = compile(*dataM, "text", threads, [data](Module &) { delete data; });
data_outputs = compile(*dataM, "text", threads, [data, &lock, &TSCtx](Module &) {
// Delete data when add_output thinks it's done with it
// Saves memory for use when multithreading
auto lock2 = std::move(lock);
delete data;
// Drop last reference to shared LLVM::Context
auto TSCtx2 = std::move(TSCtx);
});
Comment on lines +2274 to +2281
Copy link
Member

Choose a reason for hiding this comment

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

We might be able to use modern C++ features for this (though it might require replacing std::function with llvm::unique_function, which hasn't yet been standardized, so that it compiles to do the right thing)

Suggested change
data_outputs = compile(*dataM, "text", threads, [data, &lock, &TSCtx](Module &) {
// Delete data when add_output thinks it's done with it
// Saves memory for use when multithreading
auto lock2 = std::move(lock);
delete data;
// Drop last reference to shared LLVM::Context
auto TSCtx2 = std::move(TSCtx);
});
data_outputs = compile(*dataM, "text", threads, [data, lock = std::move(lock), TSCtx = std::move(TSCtx)](Module &) {
// Delete data when add_output thinks it's done with it
// Saves memory for use when multithreading
delete data;
});

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we'd need to explicitly invoke the destructor for the lambda: godbolt link

Copy link
Member

Choose a reason for hiding this comment

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

I think that is the design mistake llvm::unique_function may fix (over std::function)

}

if (params->emit_metadata) {
Expand Down