Skip to content

Commit b044d1d

Browse files
Avoid double flush hit files for collectors (#835)
Avoid double flush hit files for collectors
1 parent e1ec1cc commit b044d1d

File tree

6 files changed

+86
-63
lines changed

6 files changed

+86
-63
lines changed

Documentation/Examples/VSTest/HelloWorld/XUnitTestProject1/XUnitTestProject1.csproj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.5.0" />
1010
<PackageReference Include="xunit" Version="2.4.0" />
1111
<PackageReference Include="xunit.runner.visualstudio" Version="2.4.0" />
12-
<PackageReference Include="coverlet.collector" Version="1.2.0">
12+
<PackageReference Include="coverlet.collector" Version="1.2.1">
1313
<PrivateAssets>all</PrivateAssets>
1414
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
1515
</PackageReference>

src/coverlet.collector/InProcDataCollection/CoverletInProcDataCollector.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,8 @@ public void TestSessionEnd(TestSessionEndArgs testSessionEndArgs)
5555
{
5656
_eqtTrace.Verbose($"Calling ModuleTrackerTemplate.UnloadModule for '{injectedInstrumentationClass.Assembly.FullName}'");
5757
var unloadModule = injectedInstrumentationClass.GetMethod(nameof(ModuleTrackerTemplate.UnloadModule), new[] { typeof(object), typeof(EventArgs) });
58-
unloadModule.Invoke(null, new[] { null, EventArgs.Empty });
58+
unloadModule.Invoke(null, new[] { (object)this, EventArgs.Empty });
59+
injectedInstrumentationClass.GetField("FlushHitFile", BindingFlags.Static | BindingFlags.Public).SetValue(null, false);
5960
_eqtTrace.Verbose($"Called ModuleTrackerTemplate.UnloadModule for '{injectedInstrumentationClass.Assembly.FullName}'");
6061
}
6162
catch (Exception ex)

src/coverlet.core/Instrumentation/Instrumenter.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ internal class Instrumenter
3434
private FieldDefinition _customTrackerHitsArray;
3535
private FieldDefinition _customTrackerHitsFilePath;
3636
private FieldDefinition _customTrackerSingleHit;
37+
private FieldDefinition _customTrackerFlushHitFile;
3738
private ILProcessor _customTrackerClassConstructorIl;
3839
private TypeDefinition _customTrackerTypeDef;
3940
private MethodReference _customTrackerRegisterUnloadEventsMethod;
@@ -243,6 +244,8 @@ private void InstrumentModule()
243244
_customTrackerClassConstructorIl.InsertBefore(lastInstr, Instruction.Create(OpCodes.Stsfld, _customTrackerHitsFilePath));
244245
_customTrackerClassConstructorIl.InsertBefore(lastInstr, Instruction.Create(_singleHit ? OpCodes.Ldc_I4_1 : OpCodes.Ldc_I4_0));
245246
_customTrackerClassConstructorIl.InsertBefore(lastInstr, Instruction.Create(OpCodes.Stsfld, _customTrackerSingleHit));
247+
_customTrackerClassConstructorIl.InsertBefore(lastInstr, Instruction.Create(OpCodes.Ldc_I4_1));
248+
_customTrackerClassConstructorIl.InsertBefore(lastInstr, Instruction.Create(OpCodes.Stsfld, _customTrackerFlushHitFile));
246249

247250
if (containsAppContext)
248251
{
@@ -294,6 +297,8 @@ private void AddCustomModuleTrackerToModule(ModuleDefinition module)
294297
_customTrackerHitsFilePath = fieldClone;
295298
else if (fieldClone.Name == nameof(ModuleTrackerTemplate.SingleHit))
296299
_customTrackerSingleHit = fieldClone;
300+
else if (fieldClone.Name == nameof(ModuleTrackerTemplate.FlushHitFile))
301+
_customTrackerFlushHitFile = fieldClone;
297302
}
298303

299304
foreach (MethodDefinition methodDef in moduleTrackerTemplate.Methods)

src/coverlet.core/Instrumentation/ModuleTrackerTemplate.cs

Lines changed: 71 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ internal static class ModuleTrackerTemplate
2121
public static string HitsFilePath;
2222
public static int[] HitsArray;
2323
public static bool SingleHit;
24+
public static bool FlushHitFile;
2425
private static readonly bool _enableLog = int.TryParse(Environment.GetEnvironmentVariable("COVERLET_ENABLETRACKERLOG"), out int result) ? result == 1 : false;
2526

2627
static ModuleTrackerTemplate()
@@ -75,84 +76,95 @@ public static void RecordSingleHit(int hitLocationIndex)
7576

7677
public static void UnloadModule(object sender, EventArgs e)
7778
{
78-
try
79+
// The same module can be unloaded multiple times in the same process via different app domains.
80+
// Use a global mutex to ensure no concurrent access.
81+
using (var mutex = new Mutex(true, Path.GetFileNameWithoutExtension(HitsFilePath) + "_Mutex", out bool createdNew))
7982
{
80-
WriteLog($"Unload called for '{Assembly.GetExecutingAssembly().Location}'");
81-
// Claim the current hits array and reset it to prevent double-counting scenarios.
82-
int[] hitsArray = Interlocked.Exchange(ref HitsArray, new int[HitsArray.Length]);
83-
84-
// The same module can be unloaded multiple times in the same process via different app domains.
85-
// Use a global mutex to ensure no concurrent access.
86-
using (var mutex = new Mutex(true, Path.GetFileNameWithoutExtension(HitsFilePath) + "_Mutex", out bool createdNew))
83+
if (!createdNew)
8784
{
88-
WriteLog($"Flushing hit file '{HitsFilePath}'");
89-
if (!createdNew)
90-
mutex.WaitOne();
85+
mutex.WaitOne();
86+
}
9187

92-
bool failedToCreateNewHitsFile = false;
88+
if (FlushHitFile)
89+
{
9390
try
9491
{
95-
using (var fs = new FileStream(HitsFilePath, FileMode.CreateNew))
96-
using (var bw = new BinaryWriter(fs))
92+
// Claim the current hits array and reset it to prevent double-counting scenarios.
93+
int[] hitsArray = Interlocked.Exchange(ref HitsArray, new int[HitsArray.Length]);
94+
95+
WriteLog($"Unload called for '{Assembly.GetExecutingAssembly().Location}' by '{sender ?? "null"}'");
96+
WriteLog($"Flushing hit file '{HitsFilePath}'");
97+
98+
bool failedToCreateNewHitsFile = false;
99+
try
97100
{
98-
bw.Write(hitsArray.Length);
99-
foreach (int hitCount in hitsArray)
101+
using (var fs = new FileStream(HitsFilePath, FileMode.CreateNew))
102+
using (var bw = new BinaryWriter(fs))
100103
{
101-
bw.Write(hitCount);
104+
bw.Write(hitsArray.Length);
105+
foreach (int hitCount in hitsArray)
106+
{
107+
bw.Write(hitCount);
108+
}
102109
}
103110
}
104-
}
105-
catch (Exception ex)
106-
{
107-
WriteLog($"Failed to create new hits file '{HitsFilePath}'\n{ex}");
108-
failedToCreateNewHitsFile = true;
109-
}
110-
111-
if (failedToCreateNewHitsFile)
112-
{
113-
// Update the number of hits by adding value on disk with the ones on memory.
114-
// This path should be triggered only in the case of multiple AppDomain unloads.
115-
using (var fs = new FileStream(HitsFilePath, FileMode.Open, FileAccess.ReadWrite, FileShare.None))
116-
using (var br = new BinaryReader(fs))
117-
using (var bw = new BinaryWriter(fs))
111+
catch (Exception ex)
118112
{
119-
int hitsLength = br.ReadInt32();
120-
WriteLog($"Current hits found '{hitsLength}'");
121-
122-
if (hitsLength != hitsArray.Length)
123-
{
124-
throw new InvalidOperationException(
125-
$"{HitsFilePath} has {hitsLength} entries but on memory {nameof(HitsArray)} has {hitsArray.Length}");
126-
}
113+
WriteLog($"Failed to create new hits file '{HitsFilePath}' -> '{ex.Message}'");
114+
failedToCreateNewHitsFile = true;
115+
}
127116

128-
for (int i = 0; i < hitsLength; ++i)
117+
if (failedToCreateNewHitsFile)
118+
{
119+
// Update the number of hits by adding value on disk with the ones on memory.
120+
// This path should be triggered only in the case of multiple AppDomain unloads.
121+
using (var fs = new FileStream(HitsFilePath, FileMode.Open, FileAccess.ReadWrite, FileShare.None))
122+
using (var br = new BinaryReader(fs))
123+
using (var bw = new BinaryWriter(fs))
129124
{
130-
int oldHitCount = br.ReadInt32();
131-
bw.Seek(-sizeof(int), SeekOrigin.Current);
132-
if (SingleHit)
133-
bw.Write(hitsArray[i] + oldHitCount > 0 ? 1 : 0);
134-
else
135-
bw.Write(hitsArray[i] + oldHitCount);
125+
int hitsLength = br.ReadInt32();
126+
WriteLog($"Current hits found '{hitsLength}'");
127+
128+
if (hitsLength != hitsArray.Length)
129+
{
130+
throw new InvalidOperationException($"{HitsFilePath} has {hitsLength} entries but on memory {nameof(HitsArray)} has {hitsArray.Length}");
131+
}
132+
133+
for (int i = 0; i < hitsLength; ++i)
134+
{
135+
int oldHitCount = br.ReadInt32();
136+
bw.Seek(-sizeof(int), SeekOrigin.Current);
137+
if (SingleHit)
138+
{
139+
bw.Write(hitsArray[i] + oldHitCount > 0 ? 1 : 0);
140+
}
141+
else
142+
{
143+
bw.Write(hitsArray[i] + oldHitCount);
144+
}
145+
}
136146
}
137147
}
138-
}
139148

140-
WriteHits();
149+
WriteHits(sender);
141150

142-
// On purpose this is not under a try-finally: it is better to have an exception if there was any error writing the hits file
143-
// this case is relevant when instrumenting corelib since multiple processes can be running against the same instrumented dll.
144-
mutex.ReleaseMutex();
145-
WriteLog($"Hit file '{HitsFilePath}' flushed, size {new FileInfo(HitsFilePath).Length}");
151+
WriteLog($"Hit file '{HitsFilePath}' flushed, size {new FileInfo(HitsFilePath).Length}");
152+
WriteLog("--------------------------------");
153+
}
154+
catch (Exception ex)
155+
{
156+
WriteLog(ex.ToString());
157+
throw;
158+
}
146159
}
147-
}
148-
catch (Exception ex)
149-
{
150-
WriteLog(ex.ToString());
151-
throw;
160+
161+
// On purpose this is not under a try-finally: it is better to have an exception if there was any error writing the hits file
162+
// this case is relevant when instrumenting corelib since multiple processes can be running against the same instrumented dll.
163+
mutex.ReleaseMutex();
152164
}
153165
}
154166

155-
private static void WriteHits()
167+
private static void WriteHits(object sender)
156168
{
157169
if (_enableLog)
158170
{
@@ -172,7 +184,7 @@ private static void WriteHits()
172184
}
173185
}
174186

175-
File.AppendAllText(logFile, "Hits flushed");
187+
File.AppendAllText(logFile, $"Hits flushed file path {HitsFilePath} location '{Assembly.GetExecutingAssembly().Location}' by '{sender ?? "null"}'");
176188
}
177189
}
178190

test/coverlet.core.tests/Coverage/InstrumenterHelper.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ static class TestInstrumentationHelper
2626
/// caller sample: TestInstrumentationHelper.GenerateHtmlReport(result, sourceFileFilter: @"+**\Samples\Instrumentation.cs");
2727
/// TestInstrumentationHelper.GenerateHtmlReport(result);
2828
/// </summary>
29-
public static void GenerateHtmlReport(CoverageResult coverageResult, IReporter reporter = null, string sourceFileFilter = "", [CallerMemberName]string directory = "")
29+
public static void GenerateHtmlReport(CoverageResult coverageResult, IReporter reporter = null, string sourceFileFilter = "", [CallerMemberName] string directory = "")
3030
{
3131
JsonReporter defaultReporter = new JsonReporter();
3232
reporter ??= new CoberturaReporter();
@@ -290,5 +290,10 @@ public static void RunInProcess(this FunctionExecutor executor, Func<string[], T
290290
{
291291
Assert.Equal(0, func(args).Result);
292292
}
293+
294+
public static void RunInProcess(this FunctionExecutor executor, Func<Task<int>> func)
295+
{
296+
Assert.Equal(0, func().Result);
297+
}
293298
}
294299
}

test/coverlet.core.tests/Instrumentation/ModuleTrackerTemplateTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
using System.Threading.Tasks;
55

66
using Coverlet.Core.Instrumentation;
7-
using Coverlet.Tests.Xunit.Extensions;
87
using Xunit;
98

109
namespace Coverlet.Core.Tests.Instrumentation
@@ -14,6 +13,7 @@ class TrackerContext : IDisposable
1413
public TrackerContext()
1514
{
1615
ModuleTrackerTemplate.HitsFilePath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
16+
ModuleTrackerTemplate.FlushHitFile = true;
1717
}
1818

1919
public void Dispose()

0 commit comments

Comments
 (0)