Skip to content

Commit

Permalink
Fix assertion failure / crash in multi-core JIT
Browse files Browse the repository at this point in the history
- When the recorder times out it doesn't actually stop profiling, but writes out the profile
- The app may later stop profiling, and then it tries to write the profile again
- PR dotnet#48326 fairly expected that the profile is only written once (some state is mutated)
- The non-timeout stop-profile path was also not stopping the timer
- Fix for dotnet#53014 in main
  • Loading branch information
kouvel committed Jun 2, 2021
1 parent d49bcbe commit ac54b9d
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 18 deletions.
12 changes: 2 additions & 10 deletions src/coreclr/vm/multicorejit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1286,6 +1286,7 @@ void MulticoreJitManager::StopProfile(bool appDomainShutdown)
if (pRecorder != NULL)
{
m_fRecorderActive = false;
MulticoreJitRecorder::CloseTimer();

EX_TRY
{
Expand Down Expand Up @@ -1496,16 +1497,7 @@ void MulticoreJitManager::WriteMulticoreJitProfiler()
CONTRACTL_END;

CrstHolder hold(& m_playerLock);

// Avoid saving after MulticoreJitRecorder is deleted, and saving twice
if (!MulticoreJitRecorder::CloseTimer())
{
return;
}
if (m_pMulticoreJitRecorder != NULL)
{
m_pMulticoreJitRecorder->StopProfile(false);
}
StopProfile(false);
}
#endif // !TARGET_UNIX

Expand Down
11 changes: 3 additions & 8 deletions src/coreclr/vm/multicorejitimpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -678,20 +678,15 @@ class MulticoreJitRecorder
}

#ifndef TARGET_UNIX
static bool CloseTimer()
static void CloseTimer()
{
LIMITED_METHOD_CONTRACT;

TP_TIMER * pTimer = InterlockedExchangeT(& s_delayedWriteTimer, NULL);

if (pTimer == NULL)
if (pTimer != NULL)
{
return false;
CloseThreadpoolTimer(pTimer);
}

CloseThreadpoolTimer(pTimer);

return true;
}

~MulticoreJitRecorder()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Runtime;
using System.Runtime.CompilerServices;
using System.Threading;

public static class BasicTest
{
private static int Main()
{
const int Pass = 100;

ProfileOptimization.SetProfileRoot(Environment.CurrentDirectory);
ProfileOptimization.StartProfile("profile.mcj");

// Record a method
Foo();

// Let the multi-core JIT recorder time out. The timeout is set to 1 s in the test project.
Thread.Sleep(2000);

// Stop the profile again after timeout (just verifying that it works)
ProfileOptimization.StartProfile(null);
return Pass;
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static void Foo()
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<Optimize>true</Optimize>
<CLRTestPriority>0</CLRTestPriority>
</PropertyGroup>
<ItemGroup>
<Compile Include="McjRecorderTimeoutBeforeStop.cs" />
</ItemGroup>
<PropertyGroup>
<CLRTestBatchPreCommands><![CDATA[
$(CLRTestBatchPreCommands)
set COMPlus_MultiCoreJitProfileWriteDelay=1
]]></CLRTestBatchPreCommands>
<BashCLRTestPreCommands><![CDATA[
$(BashCLRTestPreCommands)
export COMPlus_MultiCoreJitProfileWriteDelay=1
]]></BashCLRTestPreCommands>
</PropertyGroup>
</Project>

0 comments on commit ac54b9d

Please sign in to comment.