[mapguide-internals] Death by mutex

[Trevor Wekel]

Hi everyone,

I have found a serious interaction issue in the MapGuide Server.  I did a quick check in Subversion and this issue is present in MGOS 2.1, MGOS 2.2, MGE 2010 and MGE 2011.

Basically, if you update any feature source while the MapGuide Server is under load serving pooled feature sources (ODBC, SQL Server, Oracle, etc.) then there is a risk of a deadlock occurring between the feature service cache and the Fdo connection manager.

What does this mean?  Creating or editing features sources on a live site can take it down.  How did I find it?  Would you believe a 10,000 line The Grinder script?


For those with code access, here's a pseudo stack:

Case 1: A thread has the connection manager lock and wants the feature service cache lock
MgFdoConnectionManager::Open(MgResourceIdentifier* resourceIdentifier)
  acquires MgFdoConnectionManager mutex
  if a pooled connection:
    MgFdoConnectionManager::FindFdoConnection(MgResourceIdentifier* resourceIdentifier)
      MgCacheManager::GetFeatureSourceCacheItem
        MgFeatureServiceCache::GetFeatureSource
          acquires MgFeatureServiceCache mutex

Case 2: A thread has the feature service cache lock and wants the connection manager lock MgApplicationRepositoryManager::NotifyResourceChanged
  MgCacheManager::NotifyResourceChanged(MgResourceIdentifier* resource)
    if a feature source:
      acquires MgFeatureServiceCache mutex
      MgFdoConnectionManager::RemoveCachedFdoConnection
        acquires MgFdoConnectionManager mutex


And this is the code we need to change.  If we acquire the connection manager mutex first, we should be able to avoid the deadlock. I have not done a complete code review yet so there may be other cases where this could occur.

Server\src\Common\Manager\CacheManager.cpp
void MgCacheManager::NotifyResourceChanged(CREFSTRING resource)
{
    if (STRING::npos != resource.rfind(MgResourceType::FeatureSource))
    {
        // The mutex usage and the method call order here are important
        // because they ensure all the caches are in sync.
        ACE_MT(ACE_GUARD(ACE_Recursive_Thread_Mutex, ace_mon, m_featureServiceCache.m_mutex));

        m_fdoConnectionManager->RemoveCachedFdoConnection(resource);
        m_featureServiceCache.RemoveEntry(resource);
    }
}


Regards,
Trevor