Store pinned static fields in the Pinned Object Heap.

docs/design/features/unloadability.md

@@ -11,7 +11,7 @@
 After investigating all the details, it was decided that we won't support the following scenarios in unloadable `AssemblyLoadContext` unless we get strong feedback on a need to support those.
 * Loading IJW assemblies
 * Using R2R generated code from assemblies. R2R assemblies will be loaded as plain IL ones.
-* Implementation of the FixedAddressValueTypeAttribute
+* ~~Implementation of the FixedAddressValueTypeAttribute~~ Support added in .NET TBD
 ## General scenarios
 Based on various discussions and feedback on github, the following general scenarios were often mentioned as use cases for unloadability.
 * Plugin scenarios when dynamic plugin loading and unloading is required.
@@ -84,11 +84,9 @@ We can reuse the `ComCallableWrapperCache` with only a very minor modifications
 
 The After the `AssemblyLoadContext` unload is initiated and the managed `LoaderAllocator` is collected, the `ComCallableWrapperCache` is destroyed in the `LoaderAllocator::Destroy` method.
 #### FixedAddressValueTypeAttribute for fields in collectible types
-After investigating all the details, it was decided that we won't add support for the FixedAddressValueTypeAttribute unless we get strong feedback on a need to support it.
+The fields with `FixedAddressValueTypeAttribute` are always pinned, so their address in memory never changes. Historically for non-collectible types, these fields are held pinned by a pinned `GCHandle`. But we could not use that for collectible types, since the `MethodTable` whose pointer is stored in the respective boxed instance of the value type would prevent the managed `LoaderAllocator` from being collected.
 
-If we decided to add support for it, we could do it as follows. The fields with `FixedAddressValueTypeAttribute` are always pinned, so their address in memory never changes. For non-collectible types, these fields are held pinned by a pinned `GCHandle`. But we cannot use that for collectible types, since the `MethodTable` whose pointer is stored in the respective boxed instance of the value type would prevent the managed `LoaderAllocator` from being collected.
-
-For collectible types, a new handle table can be added to `LoaderAllocator`. This handle table would be scanned during GC in a special way and all the objects the handles point to will be reported as pinned. The special scanning would be done in `Module::EnumRegularStaticGCRefs`. To pin the objects, the `promote_func` needs to be passed `GC_CALL_PINNED` in the third argument.
+Since .NET TBD, we always allocate these fields in the Pinned Object Heap. That way, they are pinned without being held by a handle, and are able to be collected.
 ## AssemblyLoadContext unloading process
 For better understanding of the unloading process, it is important to understand relations between several components that play role in the lifetime management. The picture below shows these components and the ways they reference each other.
 The green marked relations and blocks are the new ones that were added to enable unloadable `AssemblyLoadContext`. The black ones were already present before.

src/coreclr/dlls/mscorrc/mscorrc.rc

@@ -525,7 +525,6 @@ BEGIN
     IDS_CLASSLOAD_TOO_MANY_METHODS          "Type '%1' from assembly '%2' contains more methods than the current implementation allows."
     IDS_CLASSLOAD_MI_CANNOT_OVERRIDE        "Cannot override runtime implemented method '%3' on type '%1' from assembly '%2'."
 
-    IDS_CLASSLOAD_COLLECTIBLEFIXEDVTATTR   "Collectible type '%1' has unsupported FixedAddressValueTypeAttribute applied to a field."
     IDS_EE_JIT_COMPILER_ERROR               "The JIT compiler encountered invalid IL code or an internal limitation."
     IDS_EE_OBJECT_TO_VARIANT_NOT_SUPPORTED  "Invalid managed/unmanaged type combination (Marshaling to and from COM VARIANTs isn't supported)."
     IDS_EE_OBJECT_TO_ITF_NOT_SUPPORTED      "Invalid managed/unmanaged type combination (Marshaling to and from COM interface pointers isn't supported)."

src/coreclr/dlls/mscorrc/resource.h

@@ -350,7 +350,6 @@
 #define IDS_EE_OUT_OF_SYNCBLOCKS                0x1aae
 
 #define IDS_CLASSLOAD_MI_CANNOT_OVERRIDE        0x1ab3
-#define IDS_CLASSLOAD_COLLECTIBLEFIXEDVTATTR    0x1ab6
 #define IDS_CLASSLOAD_EQUIVALENTBADTYPE         0x1ab7
 #define IDS_EE_CODEEXECUTION_CONTAINSGENERICVAR 0x1abb
 #define IDS_CLASSLOAD_WRONGCPU                  0x1abc

src/coreclr/vm/gchelpers.cpp

@@ -564,7 +564,7 @@ OBJECTREF TryAllocateFrozenSzArray(MethodTable* pArrayMT, INT32 cElements)
     }
     orArray->m_NumComponents = cElements;
 
-    // Publish needs to be postponed in this case because we need to specify array length 
+    // Publish needs to be postponed in this case because we need to specify array length
     PublishObjectAndNotify(orArray, GC_ALLOC_NO_FLAGS);
 
     return ObjectToOBJECTREF(orArray);
@@ -972,7 +972,7 @@ STRINGREF AllocateString(DWORD cchStringLength, bool preferFrozenHeap, bool* pIs
         if (orString != nullptr)
         {
             orString->SetStringLength(cchStringLength);
-            // Publish needs to be postponed in this case because we need to specify string length 
+            // Publish needs to be postponed in this case because we need to specify string length
             PublishObjectAndNotify(orString, GC_ALLOC_NO_FLAGS);
             _ASSERTE(orString->GetBuffer()[cchStringLength] == W('\0'));
             orStringRef = ObjectToSTRINGREF(orString);
@@ -1024,6 +1024,7 @@ void AllocateComClassObject(ComClassFactory* pComClsFac, OBJECTREF* ppRefClass)
 // AllocateObject will throw OutOfMemoryException so don't need to check
 // for NULL return value from it.
 OBJECTREF AllocateObject(MethodTable *pMT
+                         , GC_ALLOC_FLAGS flags
 #ifdef FEATURE_COMINTEROP
                          , bool fHandleCom
 #endif
@@ -1068,7 +1069,6 @@ OBJECTREF AllocateObject(MethodTable *pMT
 #endif // FEATURE_COMINTEROP
     else
     {
-        GC_ALLOC_FLAGS flags = GC_ALLOC_NO_FLAGS;
         if (pMT->ContainsPointers())
             flags |= GC_ALLOC_CONTAINS_REF;
 

src/coreclr/vm/gchelpers.h

@@ -53,11 +53,24 @@ OBJECTREF DupArrayForCloning(BASEARRAYREF pRef);
 // for NULL return value from them.
 
 OBJECTREF AllocateObject(MethodTable *pMT
+                         , GC_ALLOC_FLAGS flags
 #ifdef FEATURE_COMINTEROP
                          , bool fHandleCom = true
 #endif
     );
 
+inline OBJECTREF AllocateObject(MethodTable *pMT
+#ifdef FEATURE_COMINTEROP
+                                , bool fHandleCom = true
+#endif
+    ) {
+    return AllocateObject(pMT, GC_ALLOC_NO_FLAGS
+#ifdef FEATURE_COMINTEROP
+                         , fHandleCom
+#endif
+        );
+    }
+
 extern int StompWriteBarrierEphemeral(bool isRuntimeSuspended);
 extern int StompWriteBarrierResize(bool isRuntimeSuspended, bool bReqUpperBoundsCheck);
 extern int SwitchToWriteWatchBarrier(bool isRuntimeSuspended);

src/coreclr/vm/generics.cpp

@@ -216,15 +216,6 @@ ClassLoader::CreateTypeHandleForNonCanonicalGenericInstantiation(
     BOOL fHasDynamicInterfaceMap = FALSE;
 #endif // FEATURE_COMINTEROP
 
-    // Collectible types have some special restrictions
-    if (pAllocator->IsCollectible())
-    {
-        if (pOldMT->HasFixedAddressVTStatics())
-        {
-            ClassLoader::ThrowTypeLoadException(pTypeKey, IDS_CLASSLOAD_COLLECTIBLEFIXEDVTATTR);
-        }
-    }
-
     // The number of bytes used for GC info
     size_t cbGC = pOldMT->ContainsPointers() ? ((CGCDesc*) pOldMT)->GetSize() : 0;
 

src/coreclr/vm/methodtable.cpp

@@ -4208,15 +4208,15 @@ void MethodTable::AllocateRegularStaticBox(FieldDesc* pField, Object** boxedStat
         {
             LOG((LF_CLASSLOADER, LL_INFO10000, "\tInstantiating static of type %s\n", pFieldMT->GetDebugClassName()));
             const bool canBeFrozen = !pFieldMT->ContainsPointers() && !Collectible();
-            OBJECTREF obj = AllocateStaticBox(pFieldMT, hasFixedAddr, NULL, canBeFrozen);
+            OBJECTREF obj = AllocateStaticBox(pFieldMT, hasFixedAddr, canBeFrozen);
             SetObjectReference((OBJECTREF*)(boxedStaticHandle), obj);
         }
     }
     GCPROTECT_END();
 }
 
 //==========================================================================================
-OBJECTREF MethodTable::AllocateStaticBox(MethodTable* pFieldMT, BOOL fPinned, OBJECTHANDLE* pHandle, bool canBeFrozen)
+OBJECTREF MethodTable::AllocateStaticBox(MethodTable* pFieldMT, BOOL fPinned, bool canBeFrozen)
 {
     CONTRACTL
     {
@@ -4236,7 +4236,6 @@ OBJECTREF MethodTable::AllocateStaticBox(MethodTable* pFieldMT, BOOL fPinned, OB
     {
         // In case if we don't plan to collect this handle we may try to allocate it on FOH
         _ASSERT(!pFieldMT->ContainsPointers());
-        _ASSERT(pHandle == nullptr);
         FrozenObjectHeapManager* foh = SystemDomain::GetFrozenObjectHeapManager();
         obj = ObjectToOBJECTREF(foh->TryAllocateObject(pFieldMT, pFieldMT->GetBaseSize()));
         // obj can be null in case if struct is huge (>64kb)
@@ -4246,25 +4245,7 @@ OBJECTREF MethodTable::AllocateStaticBox(MethodTable* pFieldMT, BOOL fPinned, OB
         }
     }
 
-    obj = AllocateObject(pFieldMT);
-
-    // Pin the object if necessary
-    if (fPinned)
-    {
-        LOG((LF_CLASSLOADER, LL_INFO10000, "\tSTATICS:Pinning static (VT fixed address attribute) of type %s\n", pFieldMT->GetDebugClassName()));
-        OBJECTHANDLE oh = GetAppDomain()->CreatePinningHandle(obj);
-        if (pHandle)
-        {
-            *pHandle = oh;
-        }
-    }
-    else
-    {
-        if (pHandle)
-        {
-            *pHandle = NULL;
-        }
-    }
+    obj = AllocateObject(pFieldMT, fPinned ? GC_ALLOC_PINNED_OBJECT_HEAP : GC_ALLOC_NO_FLAGS);
 
     return obj;
 }

src/coreclr/vm/methodtable.h

@@ -859,7 +859,7 @@ class MethodTable
 
     void AllocateRegularStaticBoxes();
     void AllocateRegularStaticBox(FieldDesc* pField, Object** boxedStaticHandle);
-    static OBJECTREF AllocateStaticBox(MethodTable* pFieldMT, BOOL fPinned, OBJECTHANDLE* pHandle = 0, bool canBeFrozen = false);
+    static OBJECTREF AllocateStaticBox(MethodTable* pFieldMT, BOOL fPinned, bool canBeFrozen = false);
 
     void CheckRestore();
 

src/coreclr/vm/methodtablebuilder.cpp

@@ -3906,11 +3906,6 @@ VOID    MethodTableBuilder::InitializeFieldDescs(FieldDesc *pFieldDescList,
             {
                 IfFailThrow(COR_E_TYPELOAD);
             }
-
-            if (bmtFP->fHasFixedAddressValueTypes && GetAssembly()->IsCollectible())
-            {
-                BuildMethodTableThrowException(IDS_CLASSLOAD_COLLECTIBLEFIXEDVTATTR);
-            }
         }
 
 

src/coreclr/vm/threads.h

@@ -192,15 +192,9 @@ struct ThreadLocalBlock
 
     ThreadStaticHandleTable * m_pThreadStaticHandleTable;
 
-    // Need to keep a list of the pinning handles we've created
-    // so they can be cleaned up when the thread dies
-    ObjectHandleList          m_PinningHandleList;
-
 public:
 
 #ifndef DACCESS_COMPILE
-    void AddPinningHandleToList(OBJECTHANDLE oh);
-    void FreePinningHandles();
     void AllocateThreadStaticHandles(Module * pModule, ThreadLocalModule * pThreadLocalModule);
     OBJECTHANDLE AllocateStaticFieldObjRefPtrs(int nRequested, OBJECTHANDLE* ppLazyAllocate = NULL);
     void InitThreadStaticHandleTable();

src/coreclr/vm/threadstatics.cpp

@@ -110,9 +110,6 @@ void ThreadLocalBlock::FreeTable()
         delete m_pThreadStaticHandleTable;
         m_pThreadStaticHandleTable = NULL;
     }
-
-    // Free any pinning handles we may have created
-    FreePinningHandles();
 }
 
 void ThreadLocalBlock::EnsureModuleIndex(ModuleIndex index)
@@ -278,37 +275,6 @@ void ThreadLocalModule::SetClassFlags(MethodTable* pMT, DWORD dwFlags)
     }
 }
 
-void ThreadLocalBlock::AddPinningHandleToList(OBJECTHANDLE oh)
-{
-    CONTRACTL
-    {
-        THROWS;
-        GC_NOTRIGGER;
-        MODE_ANY;
-    }
-    CONTRACTL_END;
-    ObjectHandleList::NodeType* pNewNode = new ObjectHandleList::NodeType(oh);
-    m_PinningHandleList.LinkHead(pNewNode);
-}
-
-void ThreadLocalBlock::FreePinningHandles()
-{
-    CONTRACTL
-    {
-        NOTHROW;
-        GC_NOTRIGGER;
-        MODE_ANY;
-    }
-    CONTRACTL_END;
-    // Destroy all pinning handles in the list, and free the nodes
-    ObjectHandleList::NodeType* pHandleNode;
-    while ((pHandleNode = m_PinningHandleList.UnlinkHead()) != NULL)
-    {
-        DestroyPinningHandle(pHandleNode->data);
-        delete pHandleNode;
-    }
-}
-
 void ThreadLocalBlock::AllocateThreadStaticHandles(Module * pModule, PTR_ThreadLocalModule pThreadLocalModule)
 {
     CONTRACTL
@@ -412,21 +378,12 @@ void ThreadLocalBlock::AllocateThreadStaticBoxes(MethodTable * pMT)
             TypeHandle  th = pField->GetFieldTypeHandleThrowing();
             MethodTable* pFieldMT = th.GetMethodTable();
 
-            // AllocateStaticBox will pin this object if this class is FixedAddressVTStatics.
-            // We save this pinning handle in a list attached to the ThreadLocalBlock. When
-            // the thread dies, we release all the pinning handles in the list.
-
-            OBJECTHANDLE handle;
-            OBJECTREF obj = MethodTable::AllocateStaticBox(pFieldMT, pMT->HasFixedAddressVTStatics(), &handle);
+            OBJECTREF obj = MethodTable::AllocateStaticBox(pFieldMT, pMT->HasFixedAddressVTStatics());
 
             PTR_BYTE pStaticBase = pMT->GetGCThreadStaticsBasePointer();
             _ASSERTE(pStaticBase != NULL);
 
             SetObjectReference( (OBJECTREF*)(pStaticBase + pField->GetOffset()), obj );
-
-            // If we created a pinning handle, save it to the list
-            if (handle != NULL)
-                AddPinningHandleToList(handle);
         }
 
         pField++;

src/tests/baseservices/compilerservices/FixedAddressValueType/FixedAddressValueType.csproj

@@ -4,8 +4,6 @@
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
     <GCStressIncompatible>true</GCStressIncompatible>
     <CLRTestPriority>1</CLRTestPriority>
-    <!-- FixedAddressValueTypeAttribute is not supported on collectible types -->
-    <UnloadabilityIncompatible>true</UnloadabilityIncompatible>
   </PropertyGroup>
   <ItemGroup>
     <Compile Include="$(MSBuildProjectName).cs" />