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

Initialize finalizer_list_marked #11820

Merged
merged 1 commit into from
Jun 23, 2015
Merged

Initialize finalizer_list_marked #11820

merged 1 commit into from
Jun 23, 2015

Conversation

yuyichao
Copy link
Contributor

Noticed this when I was checking #11814.

This doesn't cause a crash because arraylist_grow thinks the items == NULL != &_space[0] is a malloc'd array and called realloc on it (which is allowed).

The implementation seems to be robust against this but this IMHO should be fixed.

Note that this does not fix #11814

@carnaval
Copy link
Contributor

oh wow. that's bad. CI then merge. Thanks :-)

@ScottPJones
Copy link
Contributor

This is one of the things I was trying to get fixed in #11751...

@carnaval
Copy link
Contributor

Well we definitely could add some checks for this, I don't think anyone would disagree, it's a bit less of a pervasive (and apparently controversial) change than the whole diff of your PR.

@yuyichao
Copy link
Contributor Author

@ScottPJones
Copy link
Contributor

@yuyichao I hadn't yet pushed all the changes I had made, after I was told #11751 should be in separate, small commits, or PRs. I was busy working, enjoying Father's Day, and fixing """.

@StefanKarpinski
Copy link
Member

@ScottPJones, if you had identified this issue and submitted this PR – or even just an issue clearly identifying this problem – it would have been merged (or fixed) right away.

@ScottPJones
Copy link
Contributor

I was still trying to get it all to work, fixing the other initialization problem, which I did bring up in my WIP issue, but I can't do everything at once, and also keep up with my job with the Belgians.

@ScottPJones
Copy link
Contributor

Anyway, this is a very good catch, hopefully it will be merged right away.

yuyichao added a commit that referenced this pull request Jun 23, 2015
@yuyichao yuyichao merged commit d2f065e into master Jun 23, 2015
@yuyichao yuyichao deleted the yyc/finalizer_marked branch June 23, 2015 20:19
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.

readall(echo hello) leaks memory on master and 0.3
4 participants