Potential memory leak in 'shallow_clone_vec' when concurrently upgrading from vec to arc

[KillingSpark]

Hi I was interested in how this crate actually works. While reading the source I stumbled about this piece of code:

https://github.com/tokio-rs/bytes/blob/050d65b2cee8b2272687d798dc209dc03fe92719/src/bytes.rs#L1093

    match atom.compare_exchange(ptr as _, shared as _, Ordering::AcqRel, Ordering::Acquire) {
        Ok(actual) => {
            debug_assert!(actual as usize == ptr as usize);
            // The upgrade was successful, the new handle can be
            // returned.
            Bytes {
                ptr: offset,
                len,
                data: AtomicPtr::new(shared as _),
                vtable: &SHARED_VTABLE,
            }
        }
        Err(actual) => {
            // The upgrade failed, a concurrent clone happened. Release
            // the allocation that was made in this thread, it will not
            // be needed.
            let shared = Box::from_raw(shared);
            mem::forget(*shared);

            // Buffer already promoted to shared storage, so increment ref
            // count.
            shallow_clone_arc(actual as _, offset, len)
        }
    }

I am pretty sure the mem::forget should be a mem::drop so the behaviour matches the comment. forget causes the Box to never be dropped and thus the allocations will not be released.

[Darksonn]

Because of the dereference operation, the thing passed to mem::forget is a Shared without a box around it. This is to avoid running the destructor of fields in the Shared.

The box allocation is released by the dereference operation.