You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I believe there is a memory leak arising from DefaultAllocator::reallocate_copy and its use in Matrix::resize_generic. Anecdotally, I observed memory leakage in the wild in some pre-production code and narrowed its source down to here.
The resulting matrix is created by this line of resize_generic:
let res = unsafe{DefaultAllocator::reallocate_copy(new_nrows, new_ncols, data.data)};
In the case where at least one of the dimensions of the input matrix to resize_generic is Dyn, it is backed (by default) by a VecStorage; this is the type of data.data in the above line. However, in the case where the output matrix is statically sized, reallocate_copy moves the data out of this buffer to the new one, and then forgets the buffer:
// Safety:// - We don’t care about dropping elements because the caller is responsible for dropping things.// - We forget `buf` so that we don’t drop the other elements.
std::mem::forget(buf);
I believe the intention here is to ensure that the destructor does not get called on the elements of the old matrix, as the ones being used are moved to the new buffer, and the others are explicitly dropped by resize_generic; the latter part is, I believe, what is referred to by "the caller is responsible for dropping things." However, the caller cannot be responsible for dropping the buffer itself, as it is consumed by reallocate_copy; that is the source of the issue here.
I will try to put together a PR shortly to fix this and ensure that the allocation gets cleaned up, while maintaining memory safety regarding not double-dropping the matrix elements.
The text was updated successfully, but these errors were encountered:
I believe there is a memory leak arising from
DefaultAllocator::reallocate_copy
and its use inMatrix::resize_generic
. Anecdotally, I observed memory leakage in the wild in some pre-production code and narrowed its source down to here.The resulting matrix is created by this line of
resize_generic
:In the case where at least one of the dimensions of the input matrix to
resize_generic
isDyn
, it is backed (by default) by aVecStorage
; this is the type ofdata.data
in the above line. However, in the case where the output matrix is statically sized,reallocate_copy
moves the data out of this buffer to the new one, and then forgets the buffer:I believe the intention here is to ensure that the destructor does not get called on the elements of the old matrix, as the ones being used are moved to the new buffer, and the others are explicitly dropped by
resize_generic
; the latter part is, I believe, what is referred to by "the caller is responsible for dropping things." However, the caller cannot be responsible for dropping the buffer itself, as it is consumed byreallocate_copy
; that is the source of the issue here.I will try to put together a PR shortly to fix this and ensure that the allocation gets cleaned up, while maintaining memory safety regarding not double-dropping the matrix elements.
The text was updated successfully, but these errors were encountered: