Revert "Add environment variable (COMPlus_EnableDiagnostics) to disable debugging...

This reverts commit 5bcfde404803f85451cf0ee9fd6406734cb878ff.

diff --git a/src/debug/daccess/enummem.cpp b/src/debug/daccess/enummem.cpp
index 50bf408..007af0b 100644 (file)
--- a/src/debug/daccess/enummem.cpp
+++ b/src/debug/daccess/enummem.cpp
@@ -273,10 +273,10 @@ HRESULT ClrDataAccess::EnumMemCLRStatic(IN CLRDataEnumMemoryFlags flags)
     CATCH_ALL_EXCEPT_RETHROW_COR_E_OPERATIONCANCELLED( g_runtimeLoadedBaseAddress.EnumMem(); )
 #endif // !FEATURE_PAL
 
-    // These are the structures that are pointed by global pointers and we care.
-    // Some may reside in heap and some may reside as a static byte array in mscorwks.dll
-    // That is ok. We will report them explicitly.
-    //
+        // These are the structures that are pointed by global pointers and we care.
+        // Some may reside in heap and some may reside as a static byte array in mscorwks.dll
+        // That is ok. We will report them explicitly.
+        //
     CATCH_ALL_EXCEPT_RETHROW_COR_E_OPERATIONCANCELLED( g_pConfig.EnumMem(); )
     CATCH_ALL_EXCEPT_RETHROW_COR_E_OPERATIONCANCELLED( g_pPredefinedArrayTypes.EnumMem(); )
     CATCH_ALL_EXCEPT_RETHROW_COR_E_OPERATIONCANCELLED( g_pObjectClass.EnumMem(); )
@@ -295,17 +295,14 @@ HRESULT ClrDataAccess::EnumMemCLRStatic(IN CLRDataEnumMemoryFlags flags)
     CATCH_ALL_EXCEPT_RETHROW_COR_E_OPERATIONCANCELLED( g_pFreeObjectMethodTable.EnumMem(); )
     CATCH_ALL_EXCEPT_RETHROW_COR_E_OPERATIONCANCELLED( g_fHostConfig.EnumMem(); )
 
-    // These two static pointers are pointed to static data of byte[]
-    // then run constructor in place
-    //
+        // These two static pointers are pointed to static data of byte[]
+        // then run constructor in place
+        //
     CATCH_ALL_EXCEPT_RETHROW_COR_E_OPERATIONCANCELLED( SystemDomain::m_pSystemDomain.EnumMem(); )
     CATCH_ALL_EXCEPT_RETHROW_COR_E_OPERATIONCANCELLED( SharedDomain::m_pSharedDomain.EnumMem(); )
     CATCH_ALL_EXCEPT_RETHROW_COR_E_OPERATIONCANCELLED( g_pDebugger.EnumMem(); )
     CATCH_ALL_EXCEPT_RETHROW_COR_E_OPERATIONCANCELLED( g_pEEInterface.EnumMem(); )
-    if (g_pDebugInterface != nullptr)
-    {
-        CATCH_ALL_EXCEPT_RETHROW_COR_E_OPERATIONCANCELLED(g_pDebugInterface.EnumMem(); )
-    }
+    CATCH_ALL_EXCEPT_RETHROW_COR_E_OPERATIONCANCELLED( g_pDebugInterface.EnumMem(); )
     CATCH_ALL_EXCEPT_RETHROW_COR_E_OPERATIONCANCELLED( g_pEEDbgInterfaceImpl.EnumMem(); )
     CATCH_ALL_EXCEPT_RETHROW_COR_E_OPERATIONCANCELLED( g_CORDebuggerControlFlags.EnumMem(); )
     CATCH_ALL_EXCEPT_RETHROW_COR_E_OPERATIONCANCELLED( g_Mscorlib.EnumMem(); )

diff --git a/src/debug/ee/debugger.cpp b/src/debug/ee/debugger.cpp
index baefbb5..54d07cf 100644 (file)
--- a/src/debug/ee/debugger.cpp
+++ b/src/debug/ee/debugger.cpp
@@ -1885,6 +1885,29 @@ CLR_ENGINE_METRICS g_CLREngineMetrics = {
     CorDebugVersion_4_0, 
     &g_hContinueStartupEvent};
 
+
+bool IsTelestoDebugPackInstalled()
+{
+    RegKeyHolder hKey;
+    if (ERROR_SUCCESS != WszRegOpenKeyEx(HKEY_LOCAL_MACHINE, FRAMEWORK_REGISTRY_KEY_W, 0, KEY_READ, &hKey))
+        return false;
+
+    bool debugPackInstalled = false;
+
+    DWORD cbValue = 0;
+
+    if (ERROR_SUCCESS == WszRegQueryValueEx(hKey, CLRConfig::EXTERNAL_DbgPackShimPath, NULL, NULL, NULL, &cbValue))
+    {
+        if (cbValue != 0)
+        {
+            debugPackInstalled = true;
+        }
+    }
+
+    // RegCloseKey called by holder
+    return debugPackInstalled;
+}
+
 #define StartupNotifyEventNamePrefix W("TelestoStartupEvent_")
 const int cchEventNameBufferSize = sizeof(StartupNotifyEventNamePrefix)/sizeof(WCHAR) + 8; // + hex DWORD (8).  NULL terminator is included in sizeof(StartupNotifyEventNamePrefix)
 HANDLE OpenStartupNotificationEvent()
@@ -1896,7 +1919,7 @@ HANDLE OpenStartupNotificationEvent()
     return WszOpenEvent(MAXIMUM_ALLOWED | SYNCHRONIZE | EVENT_MODIFY_STATE, FALSE, szEventName);
 }
 
-void NotifyDebuggerOfStartup()
+void NotifyDebuggerOfTelestoStartup()
 {
     // Create the continue event first so that we guarantee that any
     // enumeration of this process will get back a valid continue event
@@ -1957,9 +1980,26 @@ HRESULT Debugger::Startup(void)
     _ASSERTE(g_pEEInterface != NULL);
 
 #if !defined(FEATURE_PAL)
-    // This may block while an attach occurs.
-    NotifyDebuggerOfStartup();
+    if (IsWatsonEnabled() || IsTelestoDebugPackInstalled())
+    {
+        // Iff the debug pack is installed, then go through the telesto debugging pipeline.
+        LOG((LF_CORDB, LL_INFO10, "Debugging service is enabled because debug pack is installed or Watson support is enabled)\n"));
+
+        // This may block while an attach occurs.
+        NotifyDebuggerOfTelestoStartup();
+    }
+    else
+    {
+        // On Windows, it's actually safe to finish the initialization here even without the debug pack.
+        // However, doing so causes a perf regression because we used to bail out early if the debug
+        // pack is not installed.
+        //
+        // Unlike Windows, we can't continue executing this function if the debug pack is not installed.
+        // The transport requires the debug pack to be present.  Otherwise it'll raise a fatal error.
+        return S_FALSE;
+    }
 #endif // !FEATURE_PAL
+
     {
         DebuggerLockHolder dbgLockHolder(this);
 
@@ -1970,6 +2010,7 @@ HRESULT Debugger::Startup(void)
         // threads running and throwing debug events. Keep these stress procs separate so that
         // we can focus on certain problem areas.
     #ifdef _DEBUG
+
         g_DbgShouldntUseDebugger = CLRConfig::GetConfigValue(CLRConfig::INTERNAL_DbgNoDebugger) != 0;
 
 
@@ -2037,29 +2078,6 @@ HRESULT Debugger::Startup(void)
 
         InitializeHijackFunctionAddress();
 
-        // Also initialize the AppDomainEnumerationIPCBlock
-    #if !defined(FEATURE_IPCMAN) || defined(FEATURE_DBGIPC_TRANSPORT_VM)
-        m_pAppDomainCB = new (nothrow) AppDomainEnumerationIPCBlock();
-    #else
-        m_pAppDomainCB = g_pIPCManagerInterface->GetAppDomainBlock();
-    #endif 
-
-        if (m_pAppDomainCB == NULL)
-        {
-            LOG((LF_CORDB, LL_INFO100, "D::S: Failed to get AppDomain IPC block from IPCManager.\n"));
-            ThrowHR(E_FAIL);
-        }
-
-        hr = InitAppDomainIPC();
-        _ASSERTE(SUCCEEDED(hr)); // throws on error.
-
-        // Allows the debugger (and profiler) diagnostics to be disabled so resources like 
-        // the named pipes and semaphores are not created.
-        if (CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_EnableDiagnostics) == 0)
-        {
-            return S_OK;
-        }
-
         // Create the runtime controller thread, a.k.a, the debug helper thread.
         // Don't use the interop-safe heap b/c we don't want to lazily create it.
         m_pRCThread = new DebuggerRCThread(this);
@@ -2085,6 +2103,22 @@ HRESULT Debugger::Startup(void)
 
         RaiseStartupNotification();
 
+        // Also initialize the AppDomainEnumerationIPCBlock
+    #if !defined(FEATURE_IPCMAN) || defined(FEATURE_DBGIPC_TRANSPORT_VM)
+        m_pAppDomainCB = new (nothrow) AppDomainEnumerationIPCBlock();
+    #else
+        m_pAppDomainCB = g_pIPCManagerInterface->GetAppDomainBlock();
+    #endif 
+
+        if (m_pAppDomainCB == NULL)
+        {
+            LOG((LF_CORDB, LL_INFO100, "D::S: Failed to get AppDomain IPC block from IPCManager.\n"));
+            ThrowHR(E_FAIL);
+        }
+
+        hr = InitAppDomainIPC();
+        _ASSERTE(SUCCEEDED(hr)); // throws on error.
+
         // See if we need to spin up the helper thread now, rather than later.
         DebuggerIPCControlBlock* pIPCControlBlock = m_pRCThread->GetDCB();
         (void)pIPCControlBlock; //prevent "unused variable" error from GCC
@@ -7186,8 +7220,7 @@ void Debugger::JitAttach(Thread * pThread, EXCEPTION_POINTERS * pExceptionInfo,
     }
     CONTRACTL_END;
 
-    // Don't do anything if there is a native debugger already attached or the debugging support has been disabled.
-    if (IsDebuggerPresent() || m_pRCThread == NULL)
+    if (IsDebuggerPresent())
         return;
 
     GCX_PREEMP_EEINTERFACE_TOGGLE_IFTHREAD();
@@ -14136,7 +14169,8 @@ DWORD Debugger::GetHelperThreadID(void )
 {
     LIMITED_METHOD_CONTRACT;
 
-    return m_pRCThread ? m_pRCThread->GetDCB()->m_temporaryHelperThreadId : 0;
+    return m_pRCThread->GetDCB()
+        ->m_temporaryHelperThreadId;
 }
 
 

diff --git a/src/debug/ee/debugger.h b/src/debug/ee/debugger.h
index 5eec66c..8a83cfb 100644 (file)
--- a/src/debug/ee/debugger.h
+++ b/src/debug/ee/debugger.h
@@ -3945,10 +3945,10 @@ protected:
 #if _DEBUG
 
 #define MAY_DO_HELPER_THREAD_DUTY_THROWS_CONTRACT \
-      if ((m_pRCThread == NULL) || !m_pRCThread->IsRCThreadReady()) { THROWS; } else { NOTHROW; }
+      if (!m_pRCThread->IsRCThreadReady()) { THROWS; } else { NOTHROW; }
 
 #define MAY_DO_HELPER_THREAD_DUTY_GC_TRIGGERS_CONTRACT \
-      if ((m_pRCThread == NULL) || !m_pRCThread->IsRCThreadReady() || (GetThread() != NULL)) { GC_TRIGGERS; } else { GC_NOTRIGGER; }
+      if (!m_pRCThread->IsRCThreadReady() || (GetThread() != NULL)) { GC_TRIGGERS; } else { GC_NOTRIGGER; }
 
 #define GC_TRIGGERS_FROM_GETJITINFO if (GetThreadNULLOk() != NULL) { GC_TRIGGERS; } else { GC_NOTRIGGER; }
 

diff --git a/src/inc/clrconfigvalues.h b/src/inc/clrconfigvalues.h
index 9738090..a0b205a 100644 (file)
--- a/src/inc/clrconfigvalues.h
+++ b/src/inc/clrconfigvalues.h
@@ -174,7 +174,6 @@ CONFIG_DWORD_INFO_EX(INTERNAL_BreakOnUncaughtException, W("BreakOnUncaughtExcept
 
 /// Debugger
 ///
-RETAIL_CONFIG_DWORD_INFO_EX(EXTERNAL_EnableDiagnostics, W("EnableDiagnostics"), 1, "Allows the debugger and profiler diagnostics to be disabled", CLRConfig::REGUTIL_default)
 CONFIG_DWORD_INFO_EX(INTERNAL_D__FCE, W("D::FCE"), 0, "Allows an assert when crawling the managed stack for an exception handler", CLRConfig::REGUTIL_default)
 CONFIG_DWORD_INFO_EX(INTERNAL_DbgBreakIfLocksUnavailable, W("DbgBreakIfLocksUnavailable"), 0, "Allows an assert when the debugger can't take a lock ", CLRConfig::REGUTIL_default)
 CONFIG_DWORD_INFO_EX(INTERNAL_DbgBreakOnErr, W("DbgBreakOnErr"), 0, "Allows an assert when we get a failing hresult", CLRConfig::REGUTIL_default)