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

SOMStack can cause memory leaks #173

Open
ltratt opened this issue Aug 5, 2020 · 3 comments
Open

SOMStack can cause memory leaks #173

ltratt opened this issue Aug 5, 2020 · 3 comments
Assignees

Comments

@ltratt
Copy link
Member

ltratt commented Aug 5, 2020

At the moment, SOMStack does not overwrite Vals when they are popped/truncated (instead it simply adjusts the list's length), so rboehm probably ends up keeping alive otherwise-dead objects in some cases. We could overwrite values with Val::illegal but the nicest way of doing things would be to define a custom trace function for SOMStack. CC/@jacob-hughes

[I also wonder if Vecs have the same issue?]

@ltratt ltratt self-assigned this Aug 5, 2020
@ltratt
Copy link
Member Author

ltratt commented Aug 5, 2020

I also wonder if Vecs have the same issue?

They do e.g. Vec::truncate drops items but does not overwrite the memory https://github.com/rust-lang/rust/blob/master/library/alloc/src/vec.rs#L748

@smarr
Copy link
Contributor

smarr commented Aug 5, 2020

Are there many place you use Vec? SOM doesn't have a dynamically sized data structure, I think. Except perhaps in the implementation. A quick search shows you're using it for Array

store: UnsafeCell<Vec<Val>>,

Though, arrays can't be resized in SOM, so, that shouldn't be really an issue.

@ltratt
Copy link
Member Author

ltratt commented Aug 5, 2020

The comment about Vec isn't really very yksom related: it's more something we need to think of with rboehm and friends in general. So I'm arguably guilty of raising that aspect of the problem in the wrong repo!

I have a local branch which gets rid of the Vec in Array, but we first have to convince ourselves that #172 is a good idea.

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

2 participants