Skip to content

Conversation

@bluss
Copy link
Member

@bluss bluss commented Apr 18, 2020

While this sounds tautological; to be careful, set the vector length to
zero before dropping the vector of the internal storage of an array.

In some places in ndarray where A: Copy (hence the element does not need drop) we use uninitialized elements in vectors. Setting the length to 0 avoids that the vector tries to drop, slice or otherwise produce values of these elements. (The details of the validity letting this happen with nonzero len, are under discussion as of this writing.)

Related to #796, specifically around details of Array::uninitialized safety.

The argumentation used is that we want the following to be safe:

{
    let a = Array::<bool>::uninitialized(16);
} // array `a` drops at the end of the scope

We want this to be safe even if the array has not been filled - initialized.

Under the hood we have the following operations when the array is created and dropped:

let mut v = Vec::<bool>::with_capacity(16);
v.set_len(16);

drop(v);
// Inside drop we have: {
   drop_in_place(&mut v[..]);
   <drop the actual allocation>
// }

Out of this sequence, only the drop_in_place seems problematic, and setting the vector length to 0 will avoid it. Of course, there aren't complete guarantees about Vec's behavior, but with this change we avoid a problem that has been explicitly flagged. Previously discussed in #685.

While this sounds tautological; to be careful, set the vector length to
zero before dropping the vector of the internal storage of an array.

See the comment in the code for why tihs makes a difference.
@bluss bluss merged commit ea0b69e into master Apr 18, 2020
@bluss bluss deleted the owned-repr branch April 18, 2020 13:29
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.

2 participants