Skip to content

[BUG]: DataDog.Trace.Coverage.collector > System.IO.IOException when processing DLLs to instrument #8592

Description

@Fube

Tracer Version(s)

3.43.0

Operating system and platform

Windows (x64)

Instrumentation Mode

Manual

TFM

net8.0

Bug Report

On our pipeline, we run dd-trace ci run [...] dotnet test [...] and get an error with a stack like

Data collector 'DatadogCoverage' message: Error reading the assembly: [...]
System.IO.IOException: The process cannot access the file '[...].dll' because it is being used by another process.
    at Microsoft.Win32.SafeHandles.SafeFileHandle.CreateFile(String fullPath, FileMode mode, FileAccess access, FileShare share, FileOptions options)
    at Microsoft.Win32.SafeHandles.SafeFileHandle.Open(String fullPath, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize, Nullable`1 unixCreateMode)
    at System.IO.Strategies.OSFileStreamStrategy..ctor(String path, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize, Nullable`1 unixCreateMode)
    at System.IO.Strategies.FileStreamHelpers.ChooseStrategyCore(String path, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize, Nullable`1 unixCreateMode)
    at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share)
    at Mono.Cecil.ModuleDefinition.GetFileStream(String fileName, FileMode mode, FileAccess access, FileShare share)
    at Mono.Cecil.ModuleDefinition.ReadModule(String fileName, ReaderParameters parameters)
    at Mono.Cecil.ModuleDefinition.ReadModule(String fileName)
    at Mono.Cecil.AssemblyDefinition.ReadAssembly(String fileName)

The issue is extremely consistent, with us being able to reproduce it every time we run dd-trace [...].
It also always fails on the same DLLs every time.
We used Sysinternals handle64 and found that the error message is slightly misleading: It is not another process that is holding a lock on the file, but rather the same dotnet.exe process.

We also found that there are dozens of readonly handles on the same DLLs.

We believe the issue roughly goes:
Let there be 2 projects A, B
Let A depend on B
Let A and B be picked up for processing
If A starts getting processed before B and resolves B (through tracer/src/Datadog.Trace.Coverage.collector/AssemblyProcessor.CustomResolver) before B can start getting processed, A will open a handle with FileShare.Read on B's DLL.
When B starts getting processed, it will try to open the DLL with FileAccess.ReadWrite, fail, sleep for 1 second, and then try again 2 more times.

The issue is, A's handle on B's DLL will never release.
B can retry infinitely many times, it will never be able to get RW on the DLL.

This is because when a custom AssemblyResolver is passed to Cecil in the ReaderParameters object during a call to AssemblyDefinition.ReadAssembly, Cecil marks it as Disposble.NotOwned (see: https://github.com/jbevain/cecil/blob/882ca5eedda1e62eb41bd5869aeb15d8f1538e51/Mono.Cecil/AssemblyReader.cs#L66)

That means that the using on assemblyDefinition (see:

using var assemblyDefinition = AssemblyDefinition.ReadAssembly(_assemblyFilePath, new ReaderParameters
) is not enough to release the file handles.

We are certain the issue is around this area, as we compiled a modified version of the DLL, replaced it on our machine, reran the command, and it worked flawlessly.

Our changes are threefold:
1- Traverse the dependencies of the DLLs that will be processed and create disjointed sets of DLLs so that A and B (from the above example) can not be processed in parallel
2 - Change the CustomResolver to derive from DefaultAssemblyResolver so that it can enrol custom resolved assemblies into DefaultAssemblyResolver's cache (see: https://github.com/jbevain/cecil/blob/882ca5eedda1e62eb41bd5869aeb15d8f1538e51/Mono.Cecil/DefaultAssemblyResolver.cs#L18). This fixes the dozens of file handles to the same DLL
3 - Dispose the CustomResolver instance

You can see the changes here: Fube@5c41f9d

We are unsure if you need all of these changes.
For example, the disjoint set calculation considers two projects incompatible if there is any relation between them. So, given A, B, C, where A depends on C and B depends on C, A and B are said to be incompatible.
You may not need such an aggressive isolation.

Unfortunately, we are not too sure how the IL weaving works.
Specifically, we are not certain if (in the above example) while processing A, any modification will be made to C.
We assume the answer is no (C is opened with FileAccess.Read), which would mean the disjoint set calculation can be changed to roughly "cannot process dependent and dependency at the same time" rather than "cannot process any connected assemblies, even if they are siblings (the A, B, C scenario)".

Reproduction Code

No response

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions