Move last P/Invoke error from native Thread and delete C/C++ SafeHand…

…le implementation (#100267)

* Delete native safehandle

* Delete PInvoke last error on thread

* Delete IsRealThreadPoolResetNeeded

* Delete TS_TaskReset

* Delete GetThreadContext

* Fix build break

* Delete unused resource strings

* Introduce FEATURE_IJW and use it in number of places

docs/project/glossary.md

@@ -25,6 +25,7 @@ terminology.
 | EE | [Execution Engine](https://docs.microsoft.com/dotnet/standard/managed-execution-process#running_code). |
 | GC | [Garbage Collector](https://github.com/dotnet/runtime/blob/main/docs/design/coreclr/botr/garbage-collection.md). |
 | IBC | Instrumented Block Counts - used as extension (`*.ibc`) for old PGO files. |
+| IJW | "It Just Works" - Codename for [C++/CLI](https://learn.microsoft.com/cpp/dotnet/dotnet-programming-with-cpp-cli-visual-cpp) managed/native interop |
 | IPC | Inter-Process Communication. |
 | IL | Intermediate Language. Equivalent to CIL, also equivalent to [MSIL](https://docs.microsoft.com/dotnet/standard/managed-execution-process#compiling-to-msil). |
 | JIT | [Just-in-Time](https://github.com/dotnet/runtime/blob/main/docs/design/coreclr/jit/ryujit-overview.md) compiler. RyuJIT is the code name for the next generation Just-in-Time(aka "JIT") for the .NET runtime. |

src/coreclr/clrdefinitions.cmake

@@ -92,6 +92,7 @@ if(CLR_CMAKE_TARGET_WIN32)
     add_definitions(-DFEATURE_COMINTEROP)
     add_definitions(-DFEATURE_COMINTEROP_APARTMENT_SUPPORT)
     add_definitions(-DFEATURE_COMINTEROP_UNMANAGED_ACTIVATION)
+    add_definitions(-DFEATURE_IJW) # C++/CLI managed/native interop support
 endif(CLR_CMAKE_TARGET_WIN32)
 
 add_definitions(-DFEATURE_BASICFREEZE)

src/coreclr/dlls/mscorrc/mscorrc.rc

@@ -491,9 +491,6 @@ BEGIN
     IDS_EE_CLASS_TO_VARIANT_TLB_NOT_REG     "Type '%1' cannot be marshalled to a Variant. Type library is not registered."
     IDS_EE_CANNOT_MAP_TO_MANAGED_VC         "The specified record cannot be mapped to a managed value class."
 
-    IDS_EE_SAFEHANDLECLOSED                 "Safe handle has been closed"
-    IDS_EE_SAFEHANDLECANNOTSETHANDLE        "Safe handle's handle field can only be set if the safe handle is not closed and has a ref count of 1."
-
     IDS_EE_SH_IN_VARIANT_NOT_SUPPORTED      "SafeHandle derived types cannot be stored in Variants."
 
     IDS_EE_CH_IN_VARIANT_NOT_SUPPORTED      "CriticalHandle derived types cannot be stored in Variants."

src/coreclr/dlls/mscorrc/resource.h

@@ -243,9 +243,6 @@
 #define IDS_EE_METHOD_NOT_FOUND_ON_EV_PROV      0x1a24
 #define IDS_EE_BAD_COMEVENTITF_CLASS            0x1a25
 
-#define IDS_EE_COREXEMAIN2_FAILED_TITLE         0x1a2b
-#define IDS_EE_COREXEMAIN2_FAILED_TEXT          0x1a2c
-
 #define IDS_EE_ICUSTOMMARSHALERNOTIMPL          0x1a2e
 #define IDS_EE_GETINSTANCENOTIMPL               0x1a2f
 
@@ -265,9 +262,6 @@
 #define IDS_EE_BADMARSHAL_RETURNSHCOMTONATIVE   0x1a3c
 #define IDS_EE_BADMARSHAL_SAFEHANDLE            0x1a3d
 
-#define IDS_EE_SAFEHANDLECLOSED                 0x1a3f
-#define IDS_EE_SAFEHANDLECANNOTSETHANDLE        0x1a40
-
 #define IDS_EE_BADMARSHAL_ABSTRACTRETSAFEHANDLE 0x1a44
 #define IDS_EE_SH_IN_VARIANT_NOT_SUPPORTED      0x1a47
 

src/coreclr/vm/CMakeLists.txt

@@ -358,7 +358,6 @@ set(VM_SOURCES_WKS
     reflectclasswriter.cpp
     reflectioninvocation.cpp
     runtimehandles.cpp
-    safehandle.cpp
     simplerwlock.cpp
     stackingallocator.cpp
     stringliteralmap.cpp

src/coreclr/vm/appdomain.cpp

@@ -1414,9 +1414,6 @@ void SystemDomain::LoadBaseSystemClasses()
         g_profControlBlock.fBaseSystemClassesLoaded = TRUE;
     #endif // PROFILING_SUPPORTED
 
-        // Perform any once-only SafeHandle initialization.
-        SafeHandle::Init();
-
     #if defined(_DEBUG)
         g_CoreLib.Check();
         g_CoreLib.CheckExtended();

src/coreclr/vm/callhelpers.cpp

@@ -545,9 +545,7 @@ void MethodDescCallSite::CallTargetWorker(const ARG_SLOT *pArguments, ARG_SLOT *
     if (transitionToPreemptive)
     {
         GCPreemp transitionIfILStub(transitionToPreemptive);
-        DWORD* pLastError = &GetThread()->m_dwLastErrorInterp;
         CallDescrWorkerInternal(&callDescrData);
-        *pLastError = GetLastError();
     }
     else
 #endif // FEATURE_INTERPRETER

src/coreclr/vm/comsynchronizable.h

@@ -59,7 +59,7 @@ friend class ThreadBaseObject;
     static FCDECL1(void,    Initialize,        ThreadBaseObject* pThisUNSAFE);
     static FCDECL1(FC_BOOL_RET, GetIsBackground,  ThreadBaseObject* pThisUNSAFE);
     static FCDECL1(INT32,   GetThreadState,    ThreadBaseObject* pThisUNSAFE);
-    static FCDECL1(INT32,   GetThreadContext,  ThreadBaseObject* pThisUNSAFE);
+
 #ifdef FEATURE_COMINTEROP_APARTMENT_SUPPORT
     static FCDECL1(INT32,   GetApartmentState, ThreadBaseObject* pThis);
     static FCDECL2(INT32,   SetApartmentState, ThreadBaseObject* pThisUNSAFE, INT32 iState);

src/coreclr/vm/corelib.h

@@ -188,9 +188,6 @@ DEFINE_METHOD(LICENSE_INTEROP_PROXY, SAVEKEYINCURRENTCONTEXT, SaveKeyInCurrentCo
 #endif // FEATURE_COMINTEROP
 END_ILLINK_FEATURE_SWITCH()
 
-DEFINE_CLASS_U(Interop,                CriticalHandle,             CriticalHandle)
-DEFINE_FIELD_U(handle,                     CriticalHandle,     m_handle)
-DEFINE_FIELD_U(_isClosed,                  CriticalHandle,     m_isClosed)
 DEFINE_CLASS(CRITICAL_HANDLE,       Interop,                CriticalHandle)
 DEFINE_FIELD(CRITICAL_HANDLE,       HANDLE,                 handle)
 DEFINE_METHOD(CRITICAL_HANDLE,      RELEASE_HANDLE,         ReleaseHandle,              IM_RetBool)
@@ -731,12 +728,6 @@ DEFINE_CLASS(CALLCONV_SUPPRESSGCTRANSITION,  CompilerServices,       CallConvSup
 DEFINE_CLASS(CALLCONV_MEMBERFUNCTION,        CompilerServices,       CallConvMemberFunction)
 DEFINE_CLASS(CALLCONV_SWIFT,                 CompilerServices,       CallConvSwift)
 
-DEFINE_CLASS_U(Interop,                SafeHandle,         SafeHandle)
-DEFINE_FIELD_U(_ctorStackTrace,            SafeHandle,            m_ctorStackTrace)
-DEFINE_FIELD_U(handle,                     SafeHandle,            m_handle)
-DEFINE_FIELD_U(_state,                     SafeHandle,            m_state)
-DEFINE_FIELD_U(_ownsHandle,                SafeHandle,            m_ownsHandle)
-DEFINE_FIELD_U(_fullyInitialized,          SafeHandle,            m_fullyInitialized)
 DEFINE_CLASS(SAFE_HANDLE,         Interop,                SafeHandle)
 DEFINE_FIELD(SAFE_HANDLE,           HANDLE,                 handle)
 DEFINE_METHOD(SAFE_HANDLE,          GET_IS_INVALID,         get_IsInvalid,              IM_RetBool)
@@ -895,9 +886,6 @@ DEFINE_FIELD_U(_taggedHandle,       WeakReferenceObject,    m_taggedHandle)
 DEFINE_CLASS(WEAKREFERENCE,         System,                 WeakReference)
 DEFINE_CLASS(WEAKREFERENCEGENERIC,  System,                 WeakReference`1)
 
-DEFINE_CLASS_U(Threading,           WaitHandle,             WaitHandleBase)
-DEFINE_FIELD_U(_waitHandle,         WaitHandleBase,         m_safeHandle)
-
 DEFINE_CLASS(DEBUGGER,              Diagnostics,            Debugger)
 DEFINE_METHOD(DEBUGGER,             BREAK,                  Break,                  SM_RetVoid)
 

src/coreclr/vm/dllimport.cpp

@@ -1631,9 +1631,9 @@ NDirectStubLinker::NDirectStubLinker(
     }
 #endif // FEATURE_COMINTEROP
 
-#if defined(TARGET_X86) && defined(TARGET_WINDOWS)
+#if defined(TARGET_X86) && defined(FEATURE_IJW)
     m_dwCopyCtorChainLocalNum = (DWORD)-1;
-#endif // defined(TARGET_X86) && defined(TARGET_WINDOWS)
+#endif // defined(TARGET_X86) && defined(FEATURE_IJW)
 }
 
 void NDirectStubLinker::SetCallingConvention(CorInfoCallConvExtension unmngCallConv, BOOL fIsVarArg)
@@ -1846,7 +1846,7 @@ DWORD NDirectStubLinker::GetReturnValueLocalNum()
     return m_dwRetValLocalNum;
 }
 
-#if defined(TARGET_X86) && defined(TARGET_WINDOWS)
+#if defined(TARGET_X86) && defined(FEATURE_IJW)
 DWORD NDirectStubLinker::GetCopyCtorChainLocalNum()
 {
     STANDARD_VM_CONTRACT;
@@ -1861,7 +1861,7 @@ DWORD NDirectStubLinker::GetCopyCtorChainLocalNum()
 
     return m_dwCopyCtorChainLocalNum;
 }
-#endif // defined(TARGET_X86) && defined(TARGET_WINDOWS)
+#endif // defined(TARGET_X86) && defined(FEATURE_IJW)
 
 BOOL NDirectStubLinker::IsCleanupNeeded()
 {
@@ -2179,7 +2179,7 @@ void NDirectStubLinker::DoNDirect(ILCodeStream *pcsEmit, DWORD dwStubFlags, Meth
         }
     }
 
-#if defined(TARGET_X86) && defined(TARGET_WINDOWS)
+#if defined(TARGET_X86) && defined(FEATURE_IJW)
     if (m_dwCopyCtorChainLocalNum != (DWORD)-1)
     {
         // If we have a copy constructor chain local, we need to call the copy constructor stub
@@ -2192,7 +2192,7 @@ void NDirectStubLinker::DoNDirect(ILCodeStream *pcsEmit, DWORD dwStubFlags, Meth
         pcsEmit->EmitCALL(METHOD__COPY_CONSTRUCTOR_CHAIN__INSTALL, 2, 0);
         pcsEmit->EmitLDC((DWORD_PTR)&CopyConstructorCallStub);
     }
-#endif // defined(TARGET_X86) && defined(TARGET_WINDOWS)
+#endif // defined(TARGET_X86) && defined(FEATURE_IJW)
 
     // For managed-to-native calls, the rest of the work is done by the JIT. It will
     // erect InlinedCallFrame, flip GC mode, and use the specified calling convention
@@ -2867,6 +2867,7 @@ static LPBYTE FollowIndirect(LPBYTE pTarget)
 }
 #endif // !TARGET_UNIX
 
+#ifdef FEATURE_IJW
 BOOL HeuristicDoesThisLookLikeAGetLastErrorCall(LPBYTE pTarget)
 {
     CONTRACTL
@@ -2877,7 +2878,6 @@ BOOL HeuristicDoesThisLookLikeAGetLastErrorCall(LPBYTE pTarget)
     }
     CONTRACTL_END;
 
-#if !defined(TARGET_UNIX)
     static LPBYTE pGetLastError = NULL;
     if (!pGetLastError)
     {
@@ -2912,18 +2912,10 @@ BOOL HeuristicDoesThisLookLikeAGetLastErrorCall(LPBYTE pTarget)
         // jmp [xxxx] - could be an import thunk
         return pTarget2 == pGetLastError;
     }
-#endif // !TARGET_UNIX
 
     return FALSE;
 }
-
-DWORD STDMETHODCALLTYPE FalseGetLastError()
-{
-    WRAPPER_NO_CONTRACT;
-
-    return GetThread()->m_dwLastError;
-}
-
+#endif // FEATURE_IJW
 
 CorInfoCallConvExtension GetDefaultCallConv(BOOL bIsVarArg)
 {
@@ -6141,7 +6133,7 @@ PCODE GetILStubForCalli(VASigCookie *pVASigCookie, MethodDesc *pMD)
     RETURN pVASigCookie->pNDirectILStub;
 }
 
-#if defined(TARGET_X86) && defined(TARGET_WINDOWS)
+#if defined(TARGET_X86) && defined(FEATURE_IJW)
 // Copy constructor support for C++/CLI
 EXTERN_C void* STDCALL CallCopyConstructorsWorker(void* esp)
 {
@@ -6156,6 +6148,6 @@ EXTERN_C void* STDCALL CallCopyConstructorsWorker(void* esp)
 
     return pExecute(esp);
 }
-#endif // defined(TARGET_X86) && defined(TARGET_WINDOWS)
+#endif // defined(TARGET_X86) && defined(FEATURE_IJW)
 
 #endif // #ifndef DACCESS_COMPILE

src/coreclr/vm/dllimport.h

@@ -496,9 +496,9 @@ class NDirectStubLinker : public ILStubLinker
     DWORD   GetCleanupWorkListLocalNum();
     DWORD   GetThreadLocalNum();
     DWORD   GetReturnValueLocalNum();
-#if defined(TARGET_X86) && defined(TARGET_WINDOWS)
+#if defined(TARGET_X86) && defined(FEATURE_IJW)
     DWORD   GetCopyCtorChainLocalNum();
-#endif // defined(TARGET_X86) && defined(TARGET_WINDOWS)
+#endif // defined(TARGET_X86) && defined(FEATURE_IJW)
     void    SetCleanupNeeded();
     void    SetExceptionCleanupNeeded();
     BOOL    IsCleanupWorkListSetup();
@@ -568,9 +568,9 @@ class NDirectStubLinker : public ILStubLinker
     DWORD               m_dwTargetEntryPointLocalNum;
 #endif // FEATURE_COMINTEROP
 
-#if defined(TARGET_X86) && defined(TARGET_WINDOWS)
+#if defined(TARGET_X86) && defined(FEATURE_IJW)
     DWORD               m_dwCopyCtorChainLocalNum;
-#endif // defined(TARGET_X86) && defined(TARGET_WINDOWS)
+#endif // defined(TARGET_X86) && defined(FEATURE_IJW)
 
     BOOL                m_fHasCleanupCode;
     BOOL                m_fHasExceptionCleanupCode;

src/coreclr/vm/ilmarshalers.cpp

@@ -3394,7 +3394,7 @@ ILCriticalHandleMarshaler::ReturnOverride(
     return OVERRIDDEN;
 } // ILCriticalHandleMarshaler::ReturnOverride
 
-#if defined(TARGET_WINDOWS)
+#if defined(FEATURE_IJW)
 MarshalerOverrideStatus ILBlittableValueClassWithCopyCtorMarshaler::ArgumentOverride(NDirectStubLinker* psl,
                                                 BOOL               byref,
                                                 BOOL               fin,
@@ -3520,7 +3520,7 @@ MarshalerOverrideStatus ILBlittableValueClassWithCopyCtorMarshaler::ArgumentOver
         return OVERRIDDEN;
     }
 }
-#endif // defined(TARGET_WINDOWS)
+#endif // defined(FEATURE_IJW)
 
 LocalDesc ILArgIteratorMarshaler::GetNativeType()
 {

src/coreclr/vm/ilmarshalers.h

@@ -2923,7 +2923,7 @@ class ILBlittableLayoutClassMarshaler : public ILMarshaler
     void EmitConvertContentsNativeToCLR(ILCodeStream* pslILEmit) override;
 };
 
-#if defined(TARGET_WINDOWS)
+#if defined(FEATURE_IJW)
 class ILBlittableValueClassWithCopyCtorMarshaler : public ILMarshaler
 {
 public:

src/coreclr/vm/interpreter.cpp

@@ -9271,19 +9271,6 @@ void Interpreter::DoCallWork(bool virtualCall, void* thisArg, CORINFO_RESOLVED_T
             break;
         }
 
-        // Plus some other calls that we're going to treat "like" intrinsics...
-        if (methToCall == CoreLibBinder::GetMethod(METHOD__STUBHELPERS__SET_LAST_ERROR))
-        {
-            // If we're interpreting a method that calls "SetLastError", it's very likely that the call(i) whose
-            // error we're trying to capture was performed with MethodDescCallSite machinery that itself trashes
-            // the last error.  We solve this by saving the last error in a special interpreter-specific field of
-            // "Thread" in that case, and essentially implement SetLastError here, taking that field as the
-            // source for the last error.
-            Thread* thrd = GetThread();
-            thrd->m_dwLastError = thrd->m_dwLastErrorInterp;
-            didIntrinsic = true;
-        }
-
         // TODO: The following check for hardware intrinsics is not a production-level
         //       solution and may produce incorrect results.
         static ConfigDWORD s_InterpreterHWIntrinsicsIsSupportedFalse;

src/coreclr/vm/marshalnative.cpp

@@ -34,6 +34,7 @@
 #include "comdelegate.h"
 #include "typestring.h"
 #include "appdomain.inl"
+#include "stubhelpers.h"
 
 #ifdef FEATURE_COMINTEROP
 #include "comcallablewrapper.h"
@@ -283,27 +284,47 @@ extern "C" IsInCooperativeGCMode_fn QCALLTYPE MarshalNative_GetIsInCooperativeGC
 #endif
 
 /************************************************************************
- * Marshal.GetLastPInvokeError
+ * Support for the last PInvoke error
  */
+static thread_local int t_lastPInvokeError;
+
 FCIMPL0(int, MarshalNative::GetLastPInvokeError)
 {
     FCALL_CONTRACT;
 
-    return (UINT32)(GetThread()->m_dwLastError);
+    return t_lastPInvokeError;
 }
 FCIMPLEND
 
-/************************************************************************
- * Marshal.SetLastPInvokeError
- */
 FCIMPL1(void, MarshalNative::SetLastPInvokeError, int error)
 {
     FCALL_CONTRACT;
 
-    GetThread()->m_dwLastError = (DWORD)error;
+    t_lastPInvokeError = error;
+}
+FCIMPLEND
+
+FCIMPL0(void, StubHelpers::SetLastError)
+{
+    // Make sure this is the first thing we do after returning from the target, as almost everything can cause the last error to get trashed
+    DWORD lastError = ::GetLastError();
+
+    FCALL_CONTRACT;
+
+    t_lastPInvokeError = lastError;
 }
 FCIMPLEND
 
+#ifdef FEATURE_IJW
+// GetLastError override for C++/CLI
+DWORD STDMETHODCALLTYPE FalseGetLastError()
+{
+    WRAPPER_NO_CONTRACT;
+
+    return t_lastPInvokeError;
+}
+#endif // FEATURE_IJW
+
 /************************************************************************
  * Support for the GCHandle class.
  */

src/coreclr/vm/method.cpp

@@ -3563,8 +3563,10 @@ void NDirectMethodDesc::InitEarlyBoundNDirectTarget()
     const void *target = GetModule()->GetInternalPInvokeTarget(GetRVA());
     _ASSERTE(target != 0);
 
+#ifdef FEATURE_IJW
     if (HeuristicDoesThisLookLikeAGetLastErrorCall((LPBYTE)target))
         target = (BYTE*)FalseGetLastError;
+#endif
 
     // As long as we've set the NDirect target field we don't need to backpatch the import thunk glue.
     // All NDirect calls all through the NDirect target, so if it's updated, then we won't go into