Skip to content

Conversation

@mmghannam
Copy link
Member

No description provided.

This was referenced Oct 18, 2024
@mmghannam
Copy link
Member Author

mmghannam commented Oct 18, 2024

There's a memory leak that I traced back to the tests that use model.clone_for_plugins() I can't seem to figure it out. I think the way the callbacks are implemented I'm not decrementing dropping the Arc correctly. @Andful any idea what could be wrong here?

@Andful
Copy link

Andful commented Oct 21, 2024

Yeah, looking at it seemed like there is a memory leak. From what I saw, you double Box the trait. I think once is enough. But I have to look into it deeper. I would make an issue for now, and in the future, make a different pull request for it.

@mmghannam
Copy link
Member Author

mmghannam commented Oct 21, 2024

Yeah, looking at it seemed like there is a memory leak. From what I saw, you double Box the trait. I think once is enough. But I have to look into it deeper. I would make an issue for now, and in the future, make a different pull request for it.

I think I did that because I had compilation issues without this indirection. But maybe I did something wrong back then. Anyway, I'm hesistant to merge this with this memory leak but it's limited to the use of callbacks which is not the most common use case and it will enable much safer use of the library. So let's merge it and deal with the memory leaks later as you suggested.

Update: I added issue #159 to track this memory leak.

@mmghannam mmghannam marked this pull request as ready for review October 21, 2024 14:25
@mmghannam mmghannam merged commit a311dd2 into main Oct 21, 2024
@mmghannam mmghannam deleted the arc-scipptr branch October 21, 2024 14:25
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.

3 participants