Skip to content

Commit e79ac34

Browse files
alexey-zakharovalexey-zakharov
authored
Merge pull request #286 from Unity-Technologies/fix-classunload-callback-for-generic-instances
Destroy all generic instances when module is unloaded
2 parents dd65835 + dd1a231 commit e79ac34

File tree

12 files changed

+291
-75
lines changed

12 files changed

+291
-75
lines changed

‎src/coreclr/vm/ceeload.cpp

+12-7
Original file line numberDiff line numberDiff line change
@@ -1957,7 +1957,9 @@ void Module::FreeClassTables()
19571957

19581958
if (pMT != NULL && pMT->IsRestored())
19591959
{
1960-
pMT->GetClass()->Destruct(pMT);
1960+
ClassLoader::NotifyUnload(pMT, true);
1961+
pMT->GetClass()->Destruct();
1962+
ClassLoader::NotifyUnload(pMT, false);
19611963
}
19621964
}
19631965

@@ -1973,17 +1975,20 @@ void Module::FreeClassTables()
19731975
{
19741976
TypeHandle th = pEntry->GetTypeHandle();
19751977

1976-
if (!th.IsRestored())
1978+
// Array EEClass doesn't need notification and there is no work for Destruct()
1979+
if (th.IsTypeDesc())
19771980
continue;
19781981

1982+
MethodTable * pMT = th.AsMethodTable();
1983+
ClassLoader::NotifyUnload(pMT, true);
1984+
19791985
// We need to call destruct on instances of EEClass whose "canonical" dependent lives in this table
1980-
// There is nothing interesting to destruct on array EEClass
1981-
if (!th.IsTypeDesc())
1986+
if (pMT->IsCanonicalMethodTable())
19821987
{
1983-
MethodTable * pMT = th.AsMethodTable();
1984-
if (pMT->IsCanonicalMethodTable())
1985-
pMT->GetClass()->Destruct(pMT);
1988+
pMT->GetClass()->Destruct();
19861989
}
1990+
1991+
ClassLoader::NotifyUnload(pMT, false);
19871992
}
19881993
}
19891994
}

‎src/coreclr/vm/class.cpp

+1-63
Original file line numberDiff line numberDiff line change
@@ -73,55 +73,16 @@ void *EEClass::operator new(
7373
}
7474

7575
//*******************************************************************************
76-
void EEClass::Destruct(MethodTable * pOwningMT)
76+
void EEClass::Destruct()
7777
{
7878
CONTRACTL
7979
{
8080
NOTHROW;
8181
GC_TRIGGERS;
8282
FORBID_FAULT;
83-
PRECONDITION(pOwningMT != NULL);
8483
}
8584
CONTRACTL_END
8685

87-
#ifdef PROFILING_SUPPORTED
88-
// If profiling, then notify the class is getting unloaded.
89-
{
90-
BEGIN_PROFILER_CALLBACK(CORProfilerTrackClasses());
91-
{
92-
// Calls to the profiler callback may throw, or otherwise fail, if
93-
// the profiler AVs/throws an unhandled exception/etc. We don't want
94-
// those failures to affect the runtime, so we'll ignore them.
95-
//
96-
// Note that the profiler callback may turn around and make calls into
97-
// the profiling runtime that may throw. This try/catch block doesn't
98-
// protect the profiler against such failures. To protect the profiler
99-
// against that, we will need try/catch blocks around all calls into the
100-
// profiling API.
101-
//
102-
// (Bug #26467)
103-
//
104-
105-
FAULT_NOT_FATAL();
106-
107-
EX_TRY
108-
{
109-
GCX_PREEMP();
110-
111-
(&g_profControlBlock)->ClassUnloadStarted((ClassID) pOwningMT);
112-
}
113-
EX_CATCH
114-
{
115-
// The exception here came from the profiler itself. We'll just
116-
// swallow the exception, since we don't want the profiler to bring
117-
// down the runtime.
118-
}
119-
EX_END_CATCH(RethrowTerminalExceptions);
120-
}
121-
END_PROFILER_CALLBACK();
122-
}
123-
#endif // PROFILING_SUPPORTED
124-
12586
#ifdef FEATURE_COMINTEROP
12687
// clean up any COM Data
12788
if (m_pccwTemplate)
@@ -173,29 +134,6 @@ void EEClass::Destruct(MethodTable * pOwningMT)
173134
if (GetSparseCOMInteropVTableMap() != NULL)
174135
delete GetSparseCOMInteropVTableMap();
175136
#endif // FEATURE_COMINTEROP
176-
177-
#ifdef PROFILING_SUPPORTED
178-
// If profiling, then notify the class is getting unloaded.
179-
{
180-
BEGIN_PROFILER_CALLBACK(CORProfilerTrackClasses());
181-
{
182-
// See comments in the call to ClassUnloadStarted for details on this
183-
// FAULT_NOT_FATAL marker and exception swallowing.
184-
FAULT_NOT_FATAL();
185-
EX_TRY
186-
{
187-
GCX_PREEMP();
188-
(&g_profControlBlock)->ClassUnloadFinished((ClassID) pOwningMT, S_OK);
189-
}
190-
EX_CATCH
191-
{
192-
}
193-
EX_END_CATCH(RethrowTerminalExceptions);
194-
}
195-
END_PROFILER_CALLBACK();
196-
}
197-
#endif // PROFILING_SUPPORTED
198-
199137
}
200138

201139
//*******************************************************************************

‎src/coreclr/vm/class.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -773,7 +773,7 @@ class EEClass // DO NOT CREATE A NEW EEClass USING NEW!
773773

774774
#ifndef DACCESS_COMPILE
775775
void *operator new(size_t size, LoaderHeap* pHeap, AllocMemTracker *pamTracker);
776-
void Destruct(MethodTable * pMT);
776+
void Destruct();
777777

778778
static EEClass * CreateMinimalClass(LoaderHeap *pHeap, AllocMemTracker *pamTracker);
779779
#endif // !DACCESS_COMPILE

‎src/coreclr/vm/clsload.cpp

+62-2
Original file line numberDiff line numberDiff line change
@@ -2906,7 +2906,7 @@ TypeHandle ClassLoader::DoIncrementalLoad(TypeKey *pTypeKey, TypeHandle typeHnd,
29062906

29072907
if (typeHnd.GetLoadLevel() >= CLASS_LOAD_EXACTPARENTS)
29082908
{
2909-
Notify(typeHnd);
2909+
NotifyLoad(typeHnd);
29102910
}
29112911

29122912
return typeHnd;
@@ -3109,7 +3109,7 @@ TypeHandle ClassLoader::PublishType(TypeKey *pTypeKey, TypeHandle typeHnd)
31093109
// Notify profiler and debugger that a type load has completed
31103110
// Also adjust perf counters
31113111
/*static*/
3112-
void ClassLoader::Notify(TypeHandle typeHnd)
3112+
void ClassLoader::NotifyLoad(TypeHandle typeHnd)
31133113
{
31143114
CONTRACTL
31153115
{
@@ -3177,6 +3177,66 @@ void ClassLoader::Notify(TypeHandle typeHnd)
31773177
}
31783178
}
31793179

3180+
// Notify profiler that a MethodTable is being unloaded
3181+
/*static*/
3182+
void ClassLoader::NotifyUnload(MethodTable* pMT, bool unloadStarted)
3183+
{
3184+
CONTRACTL
3185+
{
3186+
NOTHROW;
3187+
GC_TRIGGERS;
3188+
MODE_ANY;
3189+
FORBID_FAULT;
3190+
PRECONDITION(pMT != NULL);
3191+
}
3192+
CONTRACTL_END
3193+
3194+
#ifdef PROFILING_SUPPORTED
3195+
// If profiling, then notify the class is getting unloaded.
3196+
{
3197+
BEGIN_PROFILER_CALLBACK(CORProfilerTrackClasses());
3198+
{
3199+
if (pMT->ContainsGenericVariables() || pMT->IsArray())
3200+
{
3201+
// Don't notify the profiler about types with unbound variables or arrays.
3202+
// See ClassLoadStarted callback for more details.
3203+
return;
3204+
}
3205+
3206+
// Calls to the profiler callback may throw, or otherwise fail, if
3207+
// the profiler AVs/throws an unhandled exception/etc. We don't want
3208+
// those failures to affect the runtime, so we'll ignore them.
3209+
//
3210+
// Note that the profiler callback may turn around and make calls into
3211+
// the profiling runtime that may throw. This try/catch block doesn't
3212+
// protect the profiler against such failures. To protect the profiler
3213+
// against that, we will need try/catch blocks around all calls into the
3214+
// profiling API.
3215+
//
3216+
3217+
FAULT_NOT_FATAL();
3218+
3219+
EX_TRY
3220+
{
3221+
GCX_PREEMP();
3222+
3223+
if (unloadStarted)
3224+
(&g_profControlBlock)->ClassUnloadStarted((ClassID) pMT);
3225+
else
3226+
(&g_profControlBlock)->ClassUnloadFinished((ClassID) pMT, S_OK);
3227+
}
3228+
EX_CATCH
3229+
{
3230+
// The exception here came from the profiler itself. We'll just
3231+
// swallow the exception, since we don't want the profiler to bring
3232+
// down the runtime.
3233+
}
3234+
EX_END_CATCH(RethrowTerminalExceptions);
3235+
}
3236+
END_PROFILER_CALLBACK();
3237+
}
3238+
#endif // PROFILING_SUPPORTED
3239+
}
31803240

31813241
//-----------------------------------------------------------------------------
31823242
// Common helper for LoadTypeHandleForTypeKey and LoadTypeHandleForTypeKeyNoLock.

‎src/coreclr/vm/clsload.hpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -951,7 +951,9 @@ class ClassLoader
951951

952952
// Notify profiler and debugger that a type load has completed
953953
// Also update perf counters
954-
static void Notify(TypeHandle typeHnd);
954+
static void NotifyLoad(TypeHandle typeHnd);
955+
// Notify profiler that a MethodTable is being unloaded
956+
static void NotifyUnload(MethodTable* pMT, bool unloadStarted);
955957

956958
// Phase CLASS_LOAD_EXACTPARENTS of class loading
957959
// Load exact parents and interfaces and dependent structures (generics dictionary, vtable fixes)

‎src/tests/profiler/native/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ project(Profiler)
44

55
set(SOURCES
66
assemblyprofiler/assemblyprofiler.cpp
7+
classload/classload.cpp
78
eltprofiler/slowpatheltprofiler.cpp
89
eventpipeprofiler/eventpipereadingprofiler.cpp
910
eventpipeprofiler/eventpipewritingprofiler.cpp

‎src/tests/profiler/native/classfactory.cpp

+5
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "inlining/inlining.h"
2121
#include "moduleload/moduleload.h"
2222
#include "assemblyprofiler/assemblyprofiler.h"
23+
#include "classload/classload.h"
2324

2425
ClassFactory::ClassFactory(REFCLSID clsid) : refCount(0), clsid(clsid)
2526
{
@@ -139,6 +140,10 @@ HRESULT STDMETHODCALLTYPE ClassFactory::CreateInstance(IUnknown *pUnkOuter, REFI
139140
{
140141
profiler = new AssemblyProfiler();
141142
}
143+
else if (clsid == ClassLoad::GetClsid())
144+
{
145+
profiler = new ClassLoad();
146+
}
142147
else
143148
{
144149
printf("No profiler found in ClassFactory::CreateInstance. Did you add your profiler to the list?\n");

‎src/tests/profiler/native/classload/classload.cpp

+90
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
#include "classload.h"
5+
6+
GUID ClassLoad::GetClsid()
7+
{
8+
// {A1B2C3D4-E5F6-7890-1234-56789ABCDEF0}
9+
GUID clsid = {0xa1b2c3d4, 0xe5f6, 0x7890, {0x12, 0x34, 0x56, 0x78, 0x9a, 0xbc, 0xde, 0xf0}};
10+
return clsid;
11+
}
12+
13+
HRESULT ClassLoad::Initialize(IUnknown* pICorProfilerInfoUnk)
14+
{
15+
Profiler::Initialize(pICorProfilerInfoUnk);
16+
17+
HRESULT hr = S_OK;
18+
printf("Setting COR_PRF_MONITOR_CLASS_LOADS mask\n");
19+
if (FAILED(hr = pCorProfilerInfo->SetEventMask2(COR_PRF_MONITOR_CLASS_LOADS, 0)))
20+
{
21+
_failures++;
22+
printf("FAIL: ICorProfilerInfo::SetEventMask2() failed hr=0x%x", hr);
23+
return hr;
24+
}
25+
26+
return S_OK;
27+
}
28+
29+
HRESULT ClassLoad::ClassLoadStarted(ClassID classId)
30+
{
31+
_classLoadStartedCount++;
32+
return S_OK;
33+
}
34+
35+
HRESULT ClassLoad::ClassLoadFinished(ClassID classId, HRESULT hrStatus)
36+
{
37+
_classLoadFinishedCount++;
38+
return S_OK;
39+
}
40+
41+
HRESULT ClassLoad::ClassUnloadStarted(ClassID classId)
42+
{
43+
_classUnloadStartedCount++;
44+
wprintf(L"ClassUnloadStarted: %s\n", GetClassIDName(classId).ToCStr());
45+
46+
return S_OK;
47+
}
48+
49+
HRESULT ClassLoad::ClassUnloadFinished(ClassID classID, HRESULT hrStatus)
50+
{
51+
_classUnloadFinishedCount++;
52+
return S_OK;
53+
}
54+
55+
56+
HRESULT ClassLoad::Shutdown()
57+
{
58+
Profiler::Shutdown();
59+
60+
if(_failures == 0
61+
&& (_classLoadStartedCount != 0)
62+
// Expect unloading of UnloadLibrary.TestClass and
63+
// List<UnloadLibrary.TestClass> with all its base classes with everything used in List constructor:
64+
// - UnloadLibrary.TestClass
65+
// - System.Collections.Generic.IEnumerable`1<UnloadLibrary.TestClass>
66+
// - System.Collections.Generic.IList`1<UnloadLibrary.TestClass>
67+
// - System.Collections.Generic.IReadOnlyCollection`1<UnloadLibrary.TestClass>
68+
// - System.Collections.Generic.IReadOnlyList`1<UnloadLibrary.TestClass>
69+
// - System.Collections.Generic.List`1<UnloadLibrary.TestClass>
70+
// - System.Collections.Generic.ICollection`1<UnloadLibrary.TestClass>
71+
&& (_classUnloadStartedCount == 7)
72+
&& (_classLoadStartedCount == _classLoadFinishedCount)
73+
&& (_classUnloadStartedCount == _classUnloadFinishedCount))
74+
{
75+
printf("PROFILER TEST PASSES\n");
76+
}
77+
else
78+
{
79+
printf("PROFILER TEST FAILED, failures=%d classLoadStartedCount=%d classLoadFinishedCount=%d classUnloadStartedCount=%d classUnloadFinishedCount=%d\n",
80+
_failures.load(),
81+
_classLoadStartedCount.load(),
82+
_classLoadFinishedCount.load(),
83+
_classUnloadStartedCount.load(),
84+
_classUnloadFinishedCount.load());
85+
}
86+
87+
fflush(stdout);
88+
89+
return S_OK;
90+
}

‎src/tests/profiler/native/classload/classload.h

+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
#pragma once
5+
6+
#include "../profiler.h"
7+
8+
class ClassLoad : public Profiler
9+
{
10+
public:
11+
12+
ClassLoad() :
13+
Profiler(),
14+
_classLoadStartedCount(0),
15+
_classLoadFinishedCount(0),
16+
_classUnloadStartedCount(0),
17+
_classUnloadFinishedCount(0),
18+
_failures(0)
19+
{
20+
}
21+
22+
static GUID GetClsid();
23+
virtual HRESULT STDMETHODCALLTYPE Initialize(IUnknown* pICorProfilerInfoUnk);
24+
virtual HRESULT STDMETHODCALLTYPE Shutdown();
25+
26+
virtual HRESULT STDMETHODCALLTYPE ClassLoadStarted(ClassID classId);
27+
virtual HRESULT STDMETHODCALLTYPE ClassLoadFinished(ClassID classId, HRESULT hrStatus);
28+
virtual HRESULT STDMETHODCALLTYPE ClassUnloadStarted(ClassID classId);
29+
virtual HRESULT STDMETHODCALLTYPE ClassUnloadFinished(ClassID classId, HRESULT hrStatus);
30+
31+
private:
32+
std::atomic<int> _classLoadStartedCount;
33+
std::atomic<int> _classLoadFinishedCount;
34+
std::atomic<int> _classUnloadStartedCount;
35+
std::atomic<int> _classUnloadFinishedCount;
36+
std::atomic<int> _failures;
37+
};

0 commit comments

Comments
 (0)