From 23bc20633764727064484a2c8cf6416eff46435d Mon Sep 17 00:00:00 2001 From: mateoatr Date: Fri, 6 Dec 2019 23:42:52 +0000 Subject: [PATCH 1/4] Check if a UMEntryThunk has an existing handle before creating a new one. --- src/coreclr/src/vm/comdelegate.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/coreclr/src/vm/comdelegate.cpp b/src/coreclr/src/vm/comdelegate.cpp index c703333d0d93e0..a1e403b582412c 100644 --- a/src/coreclr/src/vm/comdelegate.cpp +++ b/src/coreclr/src/vm/comdelegate.cpp @@ -1307,7 +1307,9 @@ LPVOID COMDelegate::ConvertToCallback(OBJECTREF pDelegateObj) umHolder.Assign(pUMEntryThunk); // multicast. go thru Invoke - OBJECTHANDLE objhnd = GetAppDomain()->CreateLongWeakHandle(pDelegate); + OBJECTHANDLE objhnd = pUMEntryThunk->GetObjectHandle(); + if (objhnd == NULL) + objhnd = GetAppDomain()->CreateLongWeakHandle(pDelegate); _ASSERTE(objhnd != NULL); // This target should not ever be used. We are storing it in the thunk for better diagnostics of "call on collected delegate" crashes. From c29b2c432100763b10f9b94acc068be712811778 Mon Sep 17 00:00:00 2001 From: mateoatr Date: Mon, 9 Dec 2019 17:11:22 +0000 Subject: [PATCH 2/4] Destroy handle on UMEntryThunk::Terminate --- src/coreclr/src/vm/comdelegate.cpp | 4 +--- src/coreclr/src/vm/dllimportcallback.cpp | 6 ++++++ src/coreclr/src/vm/dllimportcallback.h | 16 ---------------- 3 files changed, 7 insertions(+), 19 deletions(-) diff --git a/src/coreclr/src/vm/comdelegate.cpp b/src/coreclr/src/vm/comdelegate.cpp index a1e403b582412c..c703333d0d93e0 100644 --- a/src/coreclr/src/vm/comdelegate.cpp +++ b/src/coreclr/src/vm/comdelegate.cpp @@ -1307,9 +1307,7 @@ LPVOID COMDelegate::ConvertToCallback(OBJECTREF pDelegateObj) umHolder.Assign(pUMEntryThunk); // multicast. go thru Invoke - OBJECTHANDLE objhnd = pUMEntryThunk->GetObjectHandle(); - if (objhnd == NULL) - objhnd = GetAppDomain()->CreateLongWeakHandle(pDelegate); + OBJECTHANDLE objhnd = GetAppDomain()->CreateLongWeakHandle(pDelegate); _ASSERTE(objhnd != NULL); // This target should not ever be used. We are storing it in the thunk for better diagnostics of "call on collected delegate" crashes. diff --git a/src/coreclr/src/vm/dllimportcallback.cpp b/src/coreclr/src/vm/dllimportcallback.cpp index 3960ba8b7479b7..3c8b501f7de018 100644 --- a/src/coreclr/src/vm/dllimportcallback.cpp +++ b/src/coreclr/src/vm/dllimportcallback.cpp @@ -961,12 +961,18 @@ void UMEntryThunk::Terminate() CONTRACTL { NOTHROW; + MODE_ANY; } CONTRACTL_END; m_code.Poison(); s_thunkFreeList.AddToList(this); + + if (GetObjectHandle()) + { + DestroyLongWeakHandle(GetObjectHandle()); + } } VOID UMEntryThunk::FreeUMEntryThunk(UMEntryThunk* p) diff --git a/src/coreclr/src/vm/dllimportcallback.h b/src/coreclr/src/vm/dllimportcallback.h index 6c909cb58bef6f..6fe9e6441f1a2b 100644 --- a/src/coreclr/src/vm/dllimportcallback.h +++ b/src/coreclr/src/vm/dllimportcallback.h @@ -310,22 +310,6 @@ class UMEntryThunk #endif } - ~UMEntryThunk() - { - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - MODE_ANY; - } - CONTRACTL_END; - - if (GetObjectHandle()) - { - DestroyLongWeakHandle(GetObjectHandle()); - } - } - void Terminate(); VOID RunTimeInit() From 1172956a059448dedef648a06463f7d6b4c6f479 Mon Sep 17 00:00:00 2001 From: mateoatr Date: Mon, 9 Dec 2019 19:05:19 +0000 Subject: [PATCH 3/4] Delete handle before adding thunk to free list. Set object handle to zero after destroying the handle. --- src/coreclr/src/vm/dllimportcallback.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/coreclr/src/vm/dllimportcallback.cpp b/src/coreclr/src/vm/dllimportcallback.cpp index 3c8b501f7de018..00ad4448f8be00 100644 --- a/src/coreclr/src/vm/dllimportcallback.cpp +++ b/src/coreclr/src/vm/dllimportcallback.cpp @@ -965,14 +965,15 @@ void UMEntryThunk::Terminate() } CONTRACTL_END; - m_code.Poison(); - - s_thunkFreeList.AddToList(this); - if (GetObjectHandle()) { DestroyLongWeakHandle(GetObjectHandle()); + this->m_pObjectHandle = 0; } + + m_code.Poison(); + + s_thunkFreeList.AddToList(this); } VOID UMEntryThunk::FreeUMEntryThunk(UMEntryThunk* p) From 8b4823d197c155baae16734c5d538f0e2e66ec8f Mon Sep 17 00:00:00 2001 From: mateoatr Date: Mon, 9 Dec 2019 19:30:11 +0000 Subject: [PATCH 4/4] Poison code before deleting the handle. Keep code consistent. --- src/coreclr/src/vm/dllimportcallback.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/src/vm/dllimportcallback.cpp b/src/coreclr/src/vm/dllimportcallback.cpp index 00ad4448f8be00..2f6c009d052b5d 100644 --- a/src/coreclr/src/vm/dllimportcallback.cpp +++ b/src/coreclr/src/vm/dllimportcallback.cpp @@ -965,14 +965,14 @@ void UMEntryThunk::Terminate() } CONTRACTL_END; + m_code.Poison(); + if (GetObjectHandle()) { DestroyLongWeakHandle(GetObjectHandle()); - this->m_pObjectHandle = 0; + m_pObjectHandle = 0; } - m_code.Poison(); - s_thunkFreeList.AddToList(this); }