extend_from_within leaks elements on panic

[cmazakas]

rust/library/alloc/src/vec/mod.rs

Lines 2163 to 2182 in 98f8cce

	impl<T: Clone, A: Allocator> ExtendFromWithinSpec for Vec<T, A> {
	default unsafe fn spec_extend_from_within(&mut self, src: Range<usize>) {
	let initialized = {
	let (this, spare) = self.split_at_spare_mut();
	// Safety:
	// - caller guaratees that src is a valid index
	let to_clone = unsafe { this.get_unchecked(src) };
	to_clone.iter().cloned().zip(spare.iter_mut()).map(|(e, s)| s.write(e)).count()
	};
	// Safety:
	// - elements were just initialized
	unsafe {
	let new_len = self.len() + initialized;
	self.set_len(new_len);
	}
	}
	}

Looking here, the internal length of the vector is only adjusted at the end of the loop

This has the caveat that if cloning panics, the Vec will start to unwind and only drop the first len elements which doesn't include the most recently appended ones.

Something more in line with how the stdlib does things might look like this:
https://github.com/LeonineKing1199/minivec/blob/424354dfababae7101aacf70fa9332a87e9cdb15/src/lib.rs#L1612-L1700

[jyn514]

This has the caveat that if cloning panics, the Vec will start to unwind and only drop the first len elements which doesn't include the most recently appended ones.

This is not unsafe, leaking items is safe: #24456.

[cmazakas]

This is not unsafe, leaking items is safe: #24456.

Sure; never said it was unsafe.

This is more about making sure programs who gracefully handle panics don't wind up eventually exhausting resources.

[hameerabbasi]

Assigning P-medium as part of the WG-prioritization discussion.

[WaffleLapkin]

I would like to fix this

@rustbot claim

[cmazakas]

The fix seems pretty straight-forward.

The Clone specialization just needs a RAII type that handles adjusting the Vec's new len in its Drop implementation, as I've linked in my MiniVec.

Copy can't fail so there's no need to adjust that specialization. The only thing that could theoretically panic in that case would be reallocating and in that case, there's no need to worry about the len.