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

RFC: Introduce m_foreach_allocation to lean_extern_object #2507

Closed
1 task done
SchrodingerZhu opened this issue Sep 3, 2023 · 4 comments
Closed
1 task done

RFC: Introduce m_foreach_allocation to lean_extern_object #2507

SchrodingerZhu opened this issue Sep 3, 2023 · 4 comments

Comments

@SchrodingerZhu
Copy link

Prerequisites

  • Put an X between the brackets on this line if you have done all of the following:
    • Checked that your issue isn't already filed.
    • Reduced the issue to a self-contained, reproducible test case.

Description

case LeanExternal: {
                    object * fn = lean_alloc_closure((void*)mark_persistent_fn, 1, 0);
                    lean_to_external(o)->m_class->m_foreach(lean_to_external(o)->m_data, fn);
                    lean_dec(fn);
                    break;
}

When an external object is marked as persistent, the RC pointer inside the object is recursively updated via m_foreach. However, besides the inner objects, the external object may manage other heap components. If that is the case, __lsan_ignore_object is not applied to such allocations.

I think one solution is to introduce another field, namely, m_foreach_allocation to the external object class, then we can have something like the following:

case LeanExternal: {
                    object * fn = lean_alloc_closure((void*)mark_persistent_fn, 1, 0);
                    lean_to_external(o)->m_class->m_foreach_allocation(lean_to_external(o)->m_data, __lsan_ignore_object);
                    lean_to_external(o)->m_class->m_foreach(lean_to_external(o)->m_data, fn);
                    lean_dec(fn);
                    break;
}
@tydeu
Copy link
Member

tydeu commented Sep 5, 2023

I am confused. Why can a user not just call __lsan_ignore_object on the the relevant heap components from within the custom m_foreach handler? It also has access to the m_data (as the void pointer argument), so it should be able to do it itself.

@SchrodingerZhu
Copy link
Author

Then we assume that foreach routine is only used to mark objects as persistent?

@tydeu
Copy link
Member

tydeu commented Sep 5, 2023

@SchrodingerZhu Fair point. However, if the external object does contain another lean object, you can use lean_is_persistent after apply the provided function to see if the operation being invoked is marking them persistent (instead of e.g. marking them as multithreaded). If so, then you could invoke __lsan_ignore_object on other heap components as well. Sadly, this strategy does not work on a external object that contains no internal lean objects but still has other heap components. However, you could make a dummy internal object (e.g., a unit object) as workaround to track the change the fn makes.

@Kha
Copy link
Member

Kha commented Jan 14, 2025

Thanks for the RFC. I can see how the current foreach does not cover all use cases but we're not planning on making changes to lean_extern_object at this point.

@Kha Kha closed this as not planned Won't fix, can't repro, duplicate, stale Jan 14, 2025
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

No branches or pull requests

3 participants