Add environment variable (COMPlus_EnableDiagnostics) to disable debugging and profiling #15878

mikem8361 · 2018-01-16T19:56:44Z

To disable the named pipes and semaphores created on linux execute "export COMPlus_EnableDiagnostics=0" before start the .NET Core program.

On Windows execute "set COMPlus_EnableDiagnostics=0". On Linux/OS X "export COMPlus_EnableDiagnostics=0".

The approach is to still create the DebugInterface so g_pDebugInterface != NULL but the debugger transport (g_pDbgTransport == NULL) and RC thread (m_pRCThread == NULL) are not created. This approach was used instead of not creating the DebugInterface because some of the function is used to get line number/source file info for stack traces when not debugging.

There are also some changes where g_pDebugInterface wasn't being checked for NULL that are included in the PR left over from a initial implementation. I included them to be complete because there are still some obscure cases that g_pDebugInterface == NULL like when the "debug package" isn't installed on Windows.

For issues #11769 and #8844.

jkotas · 2018-01-16T21:32:36Z

because some of the function is used to get line number/source file info for stack traces when not debugging.

These functions get the data from the VM (the actual information is managed by code manager), re-encoded it a little bit and pass it along. They should not depend on anything from the debugger stack. It should not be hard to factor them out.

I included them to be complete because there are still some obscure cases that g_pDebugInterface == NULL

What are these obscure cases? It sounds like a left over from Silverlight that can deleted. (Unless we want to make the g_pDebugInterface == NULL case work.)

mikem8361 · 2018-01-16T22:01:50Z

Noah and I looked at factoring the IL mapping code and it would take me a week or so and risk stability and maybe even perf of the lookup. This approach is simpler and the transport resources like the named pipes/semaphores and the debugger threads are not created but the IL mapping code is available without any changes. The obscure case (I probably shouldn’t have said cases) is when Watson isn’t enabled (IsWatsonEnabled()) and the Telesto debug package isn’t installed (IsTelestoDebugPackInstalled()).

jkotas · 2018-01-16T22:34:03Z

IsWatsonEnabled()

IsWatsonEnabled() is always true in CoreCLR, so this is just a dead/unrechable code in CoreCLR.

maybe even perf of the lookup

I think it would be the opposite. To get the IL->native mapping e.g. to print a stacktrace, we need to create DebuggerJitInfo today that is a completely unnecessary.

noahfalk · 2018-01-16T22:52:08Z

I think it would be the opposite. To get the IL->native mapping e.g. to print a stacktrace, we need to create DebuggerJitInfo today that is a completely unnecessary.

Our concern was in the case where you repeatedly get the same stack similar to this:
microsoft/dotnet#529
Creating the DebuggerJitInfo adds some overhead to the first request, but from then on it provides a cache of uncompressed native->IL mapping info that probably improves performance (nobody has created the alternative to benchmark it so we don't know for sure). No argument that factoring this out and creating a new cache if necessary is the architecturally cleaner solution, Mike just didn't expect it to fit within his time constraints.

jkotas · 2018-01-16T23:58:42Z

src/debug/ee/debugger.cpp

-    // RegCloseKey called by holder
-    return debugPackInstalled;
-}
-
 #define StartupNotifyEventNamePrefix W("TelestoStartupEvent_")


It is odd that this event has Telesto in the name. Does the VS debugger still depend on this event? If yes, comment maybe useful. If not, delete it.

Yeah it is still used by dbgshim.dll which VS in turn uses.
https://github.com/dotnet/coreclr/blob/master/src/dlls/dbgshim/dbgshim.cpp#L724

noahfalk · 2018-01-17T00:19:43Z

src/inc/clrconfigvalues.h

@@ -190,7 +190,8 @@ CONFIG_DWORD_INFO_EX(INTERNAL_D__FCE, W("D::FCE"), 0, "allows an assert when cra
 // 
 // Debugger
 // 
-CONFIG_DWORD_INFO_EX(INTERNAL_DbgBreakIfLocksUnavailable, W("DbgBreakIfLocksUnavailable"), 0, "allows an assert when the debugger can't take a lock ", CLRConfig::REGUTIL_default)
+RETAIL_CONFIG_DWORD_INFO_EX(INTERNAL_EnableDiagnostics, W("EnableDiagnostics"), 1, "allows the debugger and profiler diagnostics to be disabled", CLRConfig::REGUTIL_default)


According to the naming guidelines this should be EXTERNAL_EnableDiagnostics

noahfalk · 2018-01-17T00:26:27Z

src/vm/dwbucketmanager.hpp

-    }
-
-    if (m_pFaultingMD)
+    if (g_pDebugInterface != NULL)


Maybe add a comment that you aren't aware of any current scenario where g_pDebugInterface is NULL, but that we might do so in the future?

Actually I think there should be an assert that g_pDebugInterface != NULL, or simply revert the code. As-is this feels like a partial implementation. Anyone who enables g_pDebugInterface = NULL in the future needs to explicitly reconsider this code. Without more changes all the IL mappings will have been needlessly lost and that probably isn't what the future implementer will want to occur.

noahfalk · 2018-01-17T00:38:14Z

src/vm/eventtrace.cpp

@@ -5247,13 +5247,12 @@ VOID ETW::MethodLog::MethodJitted(MethodDesc *pMethodDesc, SString *namespaceOrC
        {
            // The call to SendMethodILToNativeMapEvent assumes that the debugger's lazy
            // data has already been initialized.
+			if (g_pDebugInterface != NULL)
+			{
+				g_pDebugInterface->InitializeLazyDataIfNecessary();


Same as above, a future implementor should be forced to explicitly consider what happens if they set g_pDebugInterface = NULL. Losing the ILToNative events implicitly seems bad.

noahfalk · 2018-01-17T00:39:12Z

src/vm/eventtrace.cpp

@@ -6680,16 +6679,15 @@ VOID ETW::MethodLog::SendMethodILToNativeMapEvent(MethodDesc * pMethodDesc, DWOR
    // of 64K
    const USHORT kMapEntriesMax = 7000;

+    if (g_pDebugInterface == NULL)


(same thing as above, this deserves an assert or a revert to prevent accidental loss of functionality. I'll stop calling it out)

noahfalk · 2018-01-17T00:44:03Z

src/debug/ee/debugger.cpp

+        if (CLRConfig::GetConfigValue(CLRConfig::INTERNAL_EnableDiagnostics) == 0)
+        {
+            return S_OK;
+        }


Are the jit attach cases already plumbed up to handle a missing transport? I didn't notice any changes in those code paths.

mikem8361 · 2018-01-17T01:00:25Z

Looks like I forgot about that case. Working on the change.

…ging and profiling. To disable the named pipes and semaphores created on linux execute "export COMPlus_EnableDiagnostics=0" before start the .NET Core program. On Windows execute "set COMPlus_EnableDiagnostics=0" and on Linux execute "export "COMPlus_EnableDiagnostics=0" Removed the "Telesto" registry entry (old unnecessary Silverlight code) and Watson (always true) checks. For issues #11769 and #8844.

mikem8361 · 2018-01-17T20:13:05Z

@jkotas, @noahfalk, I think I have addressed all the code review feedback. Sorry I squashed all the commits.

…le debugging and profiling. (#15878)" This reverts commit 5bcfde4.

…le debugging and profiling. (dotnet#15878)" This reverts commit 5bcfde4.

mikem8361 added the area-Diagnostics label Jan 16, 2018

mikem8361 added this to the 2.1.0 milestone Jan 16, 2018

mikem8361 self-assigned this Jan 16, 2018

mikem8361 requested a review from noahfalk January 16, 2018 19:56

mikem8361 force-pushed the disabledeb branch from 88ad6e5 to ccabfc7 Compare January 16, 2018 23:48

jkotas reviewed Jan 16, 2018

View reviewed changes

noahfalk reviewed Jan 17, 2018

View reviewed changes

mikem8361 force-pushed the disabledeb branch from ccabfc7 to 2a5c846 Compare January 17, 2018 01:46

jkotas approved these changes Jan 17, 2018

View reviewed changes

mikem8361 merged commit 5bcfde4 into dotnet:master Jan 18, 2018

mikem8361 deleted the disabledeb branch January 18, 2018 00:16

RussKeldorph mentioned this pull request Jan 25, 2018

Revert "Add environment variable (COMPlus_EnableDiagnostics) to disab… #16007

Merged

RussKeldorph added a commit that referenced this pull request Jan 25, 2018

Revert "Add environment variable (COMPlus_EnableDiagnostics) to disab…

e998512

…le debugging and profiling. (#15878)" This reverts commit 5bcfde4.

swgillespie pushed a commit to swgillespie/coreclr that referenced this pull request Jan 26, 2018

Revert "Add environment variable (COMPlus_EnableDiagnostics) to disab…

3fe9ad0

…le debugging and profiling. (dotnet#15878)" This reverts commit 5bcfde4.

jacobrillema mentioned this pull request May 15, 2018

mkfifo: cannot create fifo 'testfifo': Operation not permitted - dotnet 2.1.300 SDK CLI issues microsoft/WSL#3195

Closed

botscholar-scott mentioned this pull request Sep 27, 2019

Failed to create CoreCLR, HRESULT: 0x80004005 microsoft/python-language-server#1595

Closed

daxian-dbw mentioned this pull request Mar 23, 2020

Powershell Core not running in read-only NFS PowerShell/PowerShell#12181

Closed

daxian-dbw mentioned this pull request Jul 17, 2020

Failed to create CoreCLR, HRESULT: 0x80004005 PowerShell/PowerShell#13166

Closed

Mgamerz mentioned this pull request Dec 30, 2020

.NET 5 application on Linux fails to create CoreCLR, 0x80004005 dotnet/runtime#46462

Closed

This was referenced Nov 15, 2021

pwsh 7.20 fails to start on Windows 11 Pro Insider Preview Build PowerShell/PowerShell#16421

Closed

Failed to create CoreCLR, HRESEULT: 0x80004005 - PowerShell 7.2.0 on Windows 11 Dev PowerShell/PowerShell#16288

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add environment variable (COMPlus_EnableDiagnostics) to disable debugging and profiling #15878

Add environment variable (COMPlus_EnableDiagnostics) to disable debugging and profiling #15878

mikem8361 commented Jan 16, 2018

jkotas commented Jan 16, 2018 •

edited

Loading

mikem8361 commented Jan 16, 2018 via email

jkotas commented Jan 16, 2018

noahfalk commented Jan 16, 2018

jkotas Jan 16, 2018

noahfalk Jan 17, 2018

noahfalk Jan 17, 2018

noahfalk Jan 17, 2018

noahfalk Jan 17, 2018

noahfalk Jan 17, 2018

noahfalk Jan 17, 2018

noahfalk Jan 17, 2018

mikem8361 commented Jan 17, 2018 via email

mikem8361 commented Jan 17, 2018

Add environment variable (COMPlus_EnableDiagnostics) to disable debugging and profiling #15878

Add environment variable (COMPlus_EnableDiagnostics) to disable debugging and profiling #15878

Conversation

mikem8361 commented Jan 16, 2018

jkotas commented Jan 16, 2018 • edited Loading

mikem8361 commented Jan 16, 2018 via email

jkotas commented Jan 16, 2018

noahfalk commented Jan 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikem8361 commented Jan 17, 2018 via email

mikem8361 commented Jan 17, 2018

jkotas commented Jan 16, 2018 •

edited

Loading