Skip to content

Don't free embedded, unsized slices in their dtor #16838

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

Merged
merged 1 commit into from
Aug 29, 2014
Merged

Conversation

nrc
Copy link
Member

@nrc nrc commented Aug 29, 2014

Don't double free embedded, unsized slices.

Merge/rebase error from DST. Thanks to @eddyb for finding.

Closes #16826 (I hope)

r?

@eddyb
Copy link
Member

eddyb commented Aug 29, 2014

LGTM, modulo s/vectors/slices/.

EDIT: oops, I didn't realize you were actually handling Box<[T]> there.

@@ -63,16 +63,11 @@ pub fn make_drop_glue_unboxed<'a>(

let len = get_len(bcx, vptr);
let dataptr = get_dataptr(bcx, vptr);
let bcx = if ty::type_needs_drop(tcx, unit_ty) {
if ty::type_needs_drop(tcx, unit_ty) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like prior to your merge, the if statement actually covered the len and dataptr vars too. I don't know if that really matters.

    if ty::type_needs_drop(tcx, unit_ty) {
        let fill = get_fill(bcx, vptr);
        let dataptr = get_dataptr(bcx, vptr);
        iter_vec_raw(bcx, dataptr, unit_ty, fill, glue::drop_ty)
    } else { bcx }

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, this is calling get_fill() instead of get_len(). Curious.

Copy link
Member Author

Choose a reason for hiding this comment

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

iter_vec_raw was changed to take the length of the vec, not the fill (len*size_of_element) and get_fill was replaced with get_len

Copy link
Member Author

Choose a reason for hiding this comment

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

the len and dataptr could/should be inside the if

Thanks to @eddyb for finding the bug.

Closes rust-lang#16826 (I hope)
@nrc
Copy link
Member Author

nrc commented Aug 29, 2014

Changed approach, r?

@pnkfelix pnkfelix changed the title Don't free vectors in their dtor Don't free embedded, unsized slices in their dtor Aug 29, 2014
@nikomatsakis
Copy link
Contributor

This failure does not look related, but is worrisome in general. I have very occasionally seen this happen locally (run-pass hanging). I'm going to do a @bors: retry.

cc @alexcrichton -- have you seen these sorts of hangs before?

@alexcrichton
Copy link
Member

Yes bors has be failing quite a bit "spuriously" recently, I sadly haven't had time to investigate just yet :(

bors added a commit that referenced this pull request Aug 29, 2014
Don't double free embedded, unsized slices.

Merge/rebase error from DST. Thanks to @eddyb for finding.

Closes #16826 (I hope)

r?
@bors bors closed this Aug 29, 2014
@bors bors merged commit 415d7e8 into rust-lang:master Aug 29, 2014
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.

dst-dtor-2 is failing on the bsd snapshot bot
7 participants