Issue: If two threads try to load the same type it can cause clr to hang if the first thread to start the load happens to be a background thread. Background thread would create PendingTypeLoadEntry and insert it into unresolvedClassHash. Second thread (having normal priority) would block on PendingTypeLoadEntry->m_crst waiting for the background thread to unblock it. After doing partial load of type background thread periodically checks if there are other threads waiting for type to get loaded. In this case it does find a thread waiting so it unblocks thread. However, background thread has not removed PendingTypeLoadEntry from unresolvedClassHash. This causes the second thread to continue spinning and so not allowing background thread to get any processor cycles to remove PendingTypeLoadEntry from unresolvedClassHash. This cause app to be seem like it has hung. It is possible that this may not repro on multi-proc machine where the background thread can get scheduled on another processor when second thread continues to spin on another processor.

Fix: First remove the PendingTypeLoadEntry  from unresolvedClassHash and then unblock the waiting threads.

2 files changed, 26 insertions, 6 deletions

diff --git a/src/vm/clsload.cpp b/src/vm/clsload.cpp
index 93680d43fe..fe74bf8a6f 100644
--- a/src/vm/clsload.cpp
+++ b/src/vm/clsload.cpp
@@ -4186,10 +4186,11 @@ ClassLoader::LoadTypeHandleForTypeKey_Body(
 #endif
     }
 
-retry:
     ReleaseHolder<PendingTypeLoadEntry> pLoadingEntry;
+    CrstHolderWithState unresolvedClassLockHolder(&m_UnresolvedClassLock, false);
 
-    CrstHolderWithState unresolvedClassLockHolder(&m_UnresolvedClassLock);
+retry:
+    unresolvedClassLockHolder.Acquire();    
 
     // Is it in the hash of classes currently being loaded?
     pLoadingEntry = m_pUnresolvedClassHash->GetValue(pTypeKey);
@@ -4360,12 +4361,24 @@ retry:
         // Release the lock before proceeding. The unhandled exception filters take number of locks that
         // have ordering violations with this lock.
         unresolvedClassLockHolder.Release();
+        
+        // Unblock any thread waiting to load same type as in TypeLoadEntry
+        pLoadingEntry->UnblockWaiters();
     }
     EX_END_HOOK;
 
     // Unlink this class from the unresolved class list.
     unresolvedClassLockHolder.Acquire();
     m_pUnresolvedClassHash->DeleteValue(pTypeKey);
+    unresolvedClassLockHolder.Release();
+
+    // Unblock any thread waiting to load same type as in TypeLoadEntry. This should be done
+    // after pLoadingEntry is removed from m_pUnresolvedClassHash. Otherwise the other thread
+    // (which was waiting) will keep spinning for a while after waking up, till the current thread removes 
+    //  pLoadingEntry from m_pUnresolvedClassHash. This can cause hang in situation when the current 
+    // thread is a background thread and so will get very less processor cycle to perform subsequent
+    // operations to remove the entry from hash later. 
+    pLoadingEntry->UnblockWaiters();
 
     if (currentLevel < targetLevel)
         goto retry;
diff --git a/src/vm/pendingload.h b/src/vm/pendingload.h
index 72cb415f3a..1373215d0a 100644
--- a/src/vm/pendingload.h
+++ b/src/vm/pendingload.h
@@ -135,10 +135,6 @@ public:
             m_pException=NULL;
         }
         EX_END_CATCH(SwallowAllExceptions);
-
-        _ASSERTE(m_fLockAcquired);
-        m_Crst.Leave();
-        m_fLockAcquired = FALSE;
     }
     
     void SetResult(TypeHandle typeHnd)
@@ -153,6 +149,17 @@ public:
         CONTRACTL_END;
 
         m_typeHandle = typeHnd;
+    }
+    
+    void UnblockWaiters()
+    {
+        CONTRACTL
+        {
+              NOTHROW;
+              PRECONDITION(HasLock());
+              PRECONDITION(m_dwWaitCount > 0);
+        }
+        CONTRACTL_END;
 
         _ASSERTE(m_fLockAcquired);
         m_Crst.Leave();