Deallocate resources for DacDbiArrayList with a matching deallocator

[hoyosjs]

Fixes #57577

I deleted InfoStoreForDbiNew as there were no users.

Added DeleteDbiMemory to deal with destruction of items and proper usage of the corresponding allocator. However, using this custom new here across dll boundaries is risky as the T type will get its destructor called then the array list is destructed. The old implementation that used the CLR allocators didn't call the destructors on the inner elements (we had a potential leak, in practice it wasn't very noticeable - most objects are never deleted). After #55945, the standard array delete expression - as well as this change - call the destructor on every object. Currently, the instantiations of this type are all devoid of special destructors and are largely blittable structs, so this remains working as it was:

ICorDebugInfo::NativeVarInfo
GUID
FieldData
DebuggerIPCE_TypeArgData
DebuggerIPCE_ExpandedTypeData
DebuggerIPCE_BasicTypeData
DebuggerILToNativeMap
DacExceptionCallStackData
CordbType *
CORDB_ADDRESS
COR_SEGMENT
COR_MEMORY_RANGE

[ghost]

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Fixes #57577

I deleted InfoStoreForDbiNew as there were no users.

Added DeleteDbiMemory to deal with destruction of items and proper usage of the corresponding allocator. However, using this custom new here across dll boundaries is risky as the T type will get its destructor called then the array list is destructed. The old implementation that used the CLR allocators didn't call the destructors on the inner elements (we had a potential leak, in practice it wasn't very noticeable - most objects are never deleted). After #55945, the standard array delete expression - as well as this change - call the destructor on every object. Currently, the instantiations of this type are all devoid of special destructors and are largely blittable structs, so this remains working as it was:

ICorDebugInfo::NativeVarInfo
GUID
FieldData
DebuggerIPCE_TypeArgData
DebuggerIPCE_ExpandedTypeData
DebuggerIPCE_BasicTypeData
DebuggerILToNativeMap
DacExceptionCallStackData
CordbType *
CORDB_ADDRESS
COR_SEGMENT
COR_MEMORY_RANGE

Author:	hoyosjs
Assignees:	-
Labels:	area-Diagnostics-coreclr
Milestone:	-

[janvorli]

LGTM provided we never include src/coreclr/debug/di/rspriv.h in DAC code.

[hoyosjs]

LGTM provided we never include src/coreclr/debug/di/rspriv.h in DAC code.

The DBI and the DAC are different layers, so rspriv should never be included. That being said, I added a little safeguard.

[AaronRobinsonMSFT]

Thank you!

/cc @am11

[hoyosjs]

This codepath isn't tested in PR's (requires a debugger) so the failure is unrelated.

@dotnet/dnceng this one is weird: https://helix.dot.net/api/2019-06-17/jobs/cdc48d0e-bc58-4865-910c-7ffb0284cd9a/workitems/System.IO.Compression.ZipFile.Tests/ looks like helix ended up with a partial upload

[greenEkatherine]

I'm on it

[greenEkatherine]

zip archive is not corrupted, machine is also in good shape. it looks like an downloading error

[MattGal]

zip archive is not corrupted, machine is also in good shape. it looks like an downloading error

Two distinct machines got the exact same error though (dci-mac-build-163, dci-mac-build-008), that makes it seem unlikely to be just bad luck.

2021-09-09T00:18:34.552Z	ERROR  	executor(315)	_download_and_unpack	Exception "Error -3 while decompressing data: invalid block type" seen downloading: 
Traceback (most recent call last):
  File "/private/etc/helix/scripts/helix/executor.py", line 287, in _download_and_unpack
    self._unpack(partial_file_path, contents_root, is_workitem_payload)
  File "/private/etc/helix/scripts/helix/executor.py", line 380, in _unpack
    self._unpack_to(file_path, contents_root)
  File "/private/etc/helix/scripts/helix/executor.py", line 464, in _unpack_to
    self._unpack_zipfile(archive_path, out_dir)
  File "/private/etc/helix/scripts/helix/executor.py", line 530, in _unpack_zipfile
    zfile.extract(name, out_file_dir)
  File "/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/zipfile.py", line 1599, in extract
    return self._extract_member(member, path, pwd)
  File "/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/zipfile.py", line 1671, in _extract_member
    shutil.copyfileobj(source, target)
  File "/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/shutil.py", line 79, in copyfileobj
    buf = fsrc.read(length)
  File "/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/zipfile.py", line 899, in read
    data = self._read1(n)
  File "/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/zipfile.py", line 975, in _read1
    data = self._decompressor.decompress(data, n)
zlib.error: Error -3 while decompressing data: invalid block type
 
 Retries left:3

So Python's zipfile API thought this was a bad zip on distinct machines, making it unlikely the original download got hit by a cosmic ray. If it's interesting (you merged already so maybe not) we could examine the payload zip in greater details.

[MattGal]

Sent some details offline, the archive can't unpack stuff it got from https://github.com/dotnet/runtime-assets/blob/main/src/System.IO.Compression.TestData/ZLibTestData/WebFiles/www.reddit.com6.23.2017.har.z so there's something malformed about your work item payload in this case.

[am11]

/cc @am11

@hoyosjs, thanks for fixing DAC side of things. The remaining instances of -Wmismatched-new-delete and -Wfree-nonheap-object gcc 11 warnings in CoreCLR are fixed by: #58871 and #58885. I will look to replace gcc 9 with gcc 11 in CI's gcc leg, so we can protect this 'good state' (using their official docker as base layer, which is Ubuntu based rather than CentOS one which we currently have).