issue #194: potential race condition between creating and dropping collections

jsteemann · jsteemann · commit 9d98da43b927 · 2012-09-11T17:15:46.000+02:00
diff --git a/arangod/VocBase/vocbase.c b/arangod/VocBase/vocbase.c
@@ -187,6 +187,24 @@ static bool EqualKeyCollectionName (TRI_associative_pointer_t* array, void const
 
   return TRI_EqualString(k, e->_name);
 }
+    
+////////////////////////////////////////////////////////////////////////////////
+/// @brief remove a collection name from the global list of collections
+///
+/// this is called when a collection is dropped. it is necessary that the 
+/// caller also holds a write-lock using TRI_WRITE_LOCK_STATUS_VOCBASE_COL().
+////////////////////////////////////////////////////////////////////////////////
+
+static bool UnregisterCollection (TRI_vocbase_t* vocbase, TRI_vocbase_col_t* collection) {
+  TRI_WRITE_LOCK_COLLECTIONS_VOCBASE(vocbase);
+
+  TRI_RemoveKeyAssociativePointer(&vocbase->_collectionsByName, collection->_name);
+  TRI_RemoveKeyAssociativePointer(&vocbase->_collectionsById, &collection->_cid);
+
+  TRI_WRITE_UNLOCK_COLLECTIONS_VOCBASE(vocbase);
+
+  return true;
+}
 
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief unloads a collection
@@ -1553,14 +1571,6 @@ int TRI_UnloadCollectionVocBase (TRI_vocbase_t* vocbase, TRI_vocbase_col_t* coll
 int TRI_DropCollectionVocBase (TRI_vocbase_t* vocbase, TRI_vocbase_col_t* collection) {
   int res;
 
-  // remove name and id
-  TRI_WRITE_LOCK_COLLECTIONS_VOCBASE(vocbase);
-
-  TRI_RemoveKeyAssociativePointer(&vocbase->_collectionsByName, collection->_name);
-  TRI_RemoveKeyAssociativePointer(&vocbase->_collectionsById, &collection->_cid);
-
-  TRI_WRITE_UNLOCK_COLLECTIONS_VOCBASE(vocbase);
-
   // mark collection as deleted
   TRI_WRITE_LOCK_STATUS_VOCBASE_COL(collection);
 
@@ -1569,6 +1579,8 @@ int TRI_DropCollectionVocBase (TRI_vocbase_t* vocbase, TRI_vocbase_col_t* collec
   // .............................................................................
 
   if (collection->_status == TRI_VOC_COL_STATUS_DELETED) {
+    UnregisterCollection(vocbase, collection);
+
     TRI_WRITE_UNLOCK_STATUS_VOCBASE_COL(collection);
     return TRI_ERROR_NO_ERROR;
   }
@@ -1579,6 +1591,9 @@ int TRI_DropCollectionVocBase (TRI_vocbase_t* vocbase, TRI_vocbase_col_t* collec
 
   else if (collection->_status == TRI_VOC_COL_STATUS_NEW_BORN) {
     collection->_status = TRI_VOC_COL_STATUS_DELETED;
+    
+    UnregisterCollection(vocbase, collection);
+
     TRI_WRITE_UNLOCK_STATUS_VOCBASE_COL(collection);
     return TRI_ERROR_NO_ERROR;
   }
@@ -1620,6 +1635,8 @@ int TRI_DropCollectionVocBase (TRI_vocbase_t* vocbase, TRI_vocbase_col_t* collec
     }
 
     collection->_status = TRI_VOC_COL_STATUS_DELETED;
+    
+    UnregisterCollection(vocbase, collection);
 
     TRI_WRITE_UNLOCK_STATUS_VOCBASE_COL(collection);
 
@@ -1643,6 +1660,7 @@ int TRI_DropCollectionVocBase (TRI_vocbase_t* vocbase, TRI_vocbase_col_t* collec
     }
 
     collection->_status = TRI_VOC_COL_STATUS_DELETED;
+    UnregisterCollection(vocbase, collection);
 
     TRI_WRITE_UNLOCK_STATUS_VOCBASE_COL(collection);
 
@@ -1666,6 +1684,8 @@ int TRI_DropCollectionVocBase (TRI_vocbase_t* vocbase, TRI_vocbase_col_t* collec
 
   else {
     TRI_WRITE_UNLOCK_STATUS_VOCBASE_COL(collection);
+    LOG_WARNING("internal error in TRI_DropCollectionVocBase");
+
     return TRI_set_errno(TRI_ERROR_INTERNAL);
   }
 }
