Skip to content

Commit 92b26a4

Browse files
committed
Remove reliance on s_isTracking to detect recursion
Fixes dotnet/coreclr#21994
1 parent a50c10a commit 92b26a4

File tree

1 file changed

+10
-17
lines changed

1 file changed

+10
-17
lines changed

src/coverlet.template/ModuleTrackerTemplate.cs

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,10 @@ public static class ModuleTrackerTemplate
2020
public static string HitsFilePath;
2121
public static int[] HitsArray;
2222

23-
// Special case when instrumenting CoreLib, the static below prevents infinite loop in CoreLib
24-
// while allowing the tracker template to call any of its types and functions.
25-
private static bool s_isTracking;
26-
2723
static ModuleTrackerTemplate()
2824
{
29-
s_isTracking = true;
3025
AppDomain.CurrentDomain.ProcessExit += new EventHandler(UnloadModule);
3126
AppDomain.CurrentDomain.DomainUnload += new EventHandler(UnloadModule);
32-
s_isTracking = false;
3327

3428
// At the end of the instrumentation of a module, the instrumenter needs to add code here
3529
// to initialize the static fields according to the values derived from the instrumentation of
@@ -38,15 +32,18 @@ static ModuleTrackerTemplate()
3832

3933
public static void RecordHit(int hitLocationIndex)
4034
{
41-
if (s_isTracking)
35+
// Make sure to avoid recording if this is a call to RecordHit within the AppDomain setup code in an
36+
// instrumented build of System.Private.CoreLib.
37+
if (HitsArray is null)
4238
return;
4339

4440
Interlocked.Increment(ref HitsArray[hitLocationIndex]);
4541
}
4642

4743
public static void UnloadModule(object sender, EventArgs e)
4844
{
49-
s_isTracking = true;
45+
// Claim the current hits array and reset it to prevent double-counting scenarios.
46+
var hitsArray = Interlocked.Exchange(ref HitsArray, new int[HitsArray.Length]);
5047

5148
// The same module can be unloaded multiple times in the same process via different app domains.
5249
// Use a global mutex to ensure no concurrent access.
@@ -61,8 +58,8 @@ public static void UnloadModule(object sender, EventArgs e)
6158
using (var fs = new FileStream(HitsFilePath, FileMode.CreateNew))
6259
using (var bw = new BinaryWriter(fs))
6360
{
64-
bw.Write(HitsArray.Length);
65-
foreach (int hitCount in HitsArray)
61+
bw.Write(hitsArray.Length);
62+
foreach (int hitCount in hitsArray)
6663
{
6764
bw.Write(hitCount);
6865
}
@@ -82,25 +79,21 @@ public static void UnloadModule(object sender, EventArgs e)
8279
using (var bw = new BinaryWriter(fs))
8380
{
8481
int hitsLength = br.ReadInt32();
85-
if (hitsLength != HitsArray.Length)
82+
if (hitsLength != hitsArray.Length)
8683
{
8784
throw new InvalidOperationException(
88-
$"{HitsFilePath} has {hitsLength} entries but on memory {nameof(HitsArray)} has {HitsArray.Length}");
85+
$"{HitsFilePath} has {hitsLength} entries but on memory {nameof(HitsArray)} has {hitsArray.Length}");
8986
}
9087

9188
for (int i = 0; i < hitsLength; ++i)
9289
{
9390
int oldHitCount = br.ReadInt32();
9491
bw.Seek(-sizeof(int), SeekOrigin.Current);
95-
bw.Write(HitsArray[i] + oldHitCount);
92+
bw.Write(hitsArray[i] + oldHitCount);
9693
}
9794
}
9895
}
9996

100-
// Prevent any double counting scenario, i.e.: UnloadModule called twice (not sure if this can happen in practice ...)
101-
// Only an issue if DomainUnload and ProcessExit can both happens, perhaps can be removed...
102-
Array.Clear(HitsArray, 0, HitsArray.Length);
103-
10497
// 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
10598
// this case is relevant when instrumenting corelib since multiple processes can be running against the same instrumented dll.
10699
mutex.ReleaseMutex();

0 commit comments

Comments
 (0)