Fix ArrayBuffer memory leak

[overlookmotel]

Fix for #1419.

Problem

Problem is that Uint8Array::new only records the length of the Vec it receives, and ignores the capacity.

When it recreates the Vec later in order to drop it, it is created with capacity equal to the length:

napi-rs/crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs

Lines 109 to 110 in 83e157e

	let length = self.length;
	unsafe { Vec::from_raw_parts(self.data, length, length) };

So if original Vec had a larger capacity, capacity - length bytes are leaked. The allocator is also called with a different layout from what the original Vec was created with, which may cause unknown consequences.

Solution

This PR just adds data::shrink_to_fit() before recording length of the Vec. This is the simplest solution, and since Uint8Array::new takes ownership of the Vec, I can't see much benefit of not freeing the excess capacity immediately.

Potential complication

As far as I can see, Vec::shrink_to_fit() always reduces capacity to exactly length:

https://github.com/rust-lang/rust/blob/8ea62f6c30c94778d9146ddc26c8520ed2b50a58/library/alloc/src/raw_vec.rs#L425-L440

However, Rust's docs say "It will drop down as close as possible to the length but the allocator may still inform the vector that there is space for a few more elements."

So can we rely on shrink_to_fit() always leaving capacity == length?

If not, would need to record capacity in the Uint8Array, and then use that value when dropping (as Buffer does).

If you think this is a concern, please let me know and I'll amend this PR.

[overlookmotel]

Some of the CI fails are due to unrelated errors, but one test is failing on several platforms:

napi7 › arraybuffer › is detached arraybuffer should work fine

I'll look into that.

[Brooooooklyn]

@overlookmotel thanks for contributing, I'll merge it and fix the failed tests in a separate pull request

[overlookmotel]

Thanks for merging.

Are you not concerned about not being able to rely on Vec::shrink_to_fit() always resulting in capacity == length (either now or in the future)?

I was tending towards thinking that it'd be safer to add a capacity field to make absolutely sure and was going to update this PR. I'd be happy to do a follow-up PR to do that if you agree it'd be a good idea.

[Brooooooklyn]

I was tending towards thinking that it'd be safer to add a capacity field to make absolutely sure and was going to update this PR.

Sure, thanks!