Fix memory leaks around LogicalCollection.

arangod/V8Server/v8-collection-util.cpp

@@ -109,8 +109,11 @@ static void WeakCollectionCallback(const v8::WeakCallbackData<
   }
 }
 
-/////////////////////////////////////////////////////////////////////////////////
+////////////////////////////////////////////////////////////////////////////////
 /// @brief wraps a LogicalCollection
+/// Note that if collection is a local collection, then the object will never
+/// be freed. If it is not a local collection (coordinator case), then delete
+/// will be called when the V8 object is garbage collected.
 ////////////////////////////////////////////////////////////////////////////////
 
 v8::Handle<v8::Object> WrapCollection(v8::Isolate* isolate,

arangod/V8Server/v8-collection.cpp

@@ -178,7 +178,8 @@ static int ParseDocumentOrDocumentHandle(v8::Isolate* isolate,
       try {
         std::shared_ptr<LogicalCollection> col =
             ci->getCollection(vocbase->name(), collectionName);
-        collection = col->clone();
+        auto colCopy = col->clone();
+        collection = colCopy.release();
       } catch (...) {
         return TRI_ERROR_ARANGO_COLLECTION_NOT_FOUND;
       }
@@ -2533,7 +2534,8 @@ static void JS_CollectionVocbase(
     try {
       std::shared_ptr<LogicalCollection> const ci =
           ClusterInfo::instance()->getCollection(vocbase->name(), name);
-      collection = ci->clone();
+      auto colCopy = ci->clone();
+      collection = colCopy.release();
     } catch (...) {
       // not found
       TRI_V8_RETURN_NULL();

arangod/V8Server/v8-collection.h

@@ -46,7 +46,12 @@ bool EqualCollection(arangodb::CollectionNameResolver const* resolver,
                      std::string const& collectionName,
                      arangodb::LogicalCollection const* collection);
 
+////////////////////////////////////////////////////////////////////////////////
 /// @brief wraps a LogicalCollection
+/// Note that if collection is a local collection, then the object will never
+/// be freed. If it is not a local collection (coordinator case), then delete
+/// will be called when the V8 object is garbage collected.
+////////////////////////////////////////////////////////////////////////////////
 
 v8::Handle<v8::Object> WrapCollection(
     v8::Isolate* isolate, arangodb::LogicalCollection const* collection);

arangod/V8Server/v8-vocbase.cpp

@@ -1869,7 +1869,8 @@ static void MapGetVocBase(v8::Local<v8::String> const name,
     if (ServerState::instance()->isCoordinator()) {
       auto ci = ClusterInfo::instance()->getCollection(vocbase->name(),
                                                        std::string(key));
-      collection = ci->clone();
+      auto colCopy = ci->clone();
+      collection = colCopy.release();  // will be delete on garbage collection
     } else {
       collection = vocbase->lookupCollection(std::string(key));
     }

arangod/V8Server/v8-vocindex.cpp

@@ -633,7 +633,7 @@ static void EnsureIndex(v8::FunctionCallbackInfo<v8::Value> const& args,
 /// @brief create a collection on the coordinator
 ////////////////////////////////////////////////////////////////////////////////
 
-LogicalCollection* CreateCollectionCoordinator(LogicalCollection* parameters) {
+std::unique_ptr<LogicalCollection> CreateCollectionCoordinator(LogicalCollection* parameters) {
   std::string distributeShardsLike = parameters->distributeShardsLike();
 
   std::vector<std::string> dbServers;
@@ -664,8 +664,6 @@ LogicalCollection* CreateCollectionCoordinator(LogicalCollection* parameters) {
       parameters->distributeShardsLike(otherCidString);
     }
   }
-
-  
   
   // If the list dbServers is still empty, it will be filled in
   // distributeShards below.
@@ -699,8 +697,7 @@ LogicalCollection* CreateCollectionCoordinator(LogicalCollection* parameters) {
   // collection does not exist. Also, the create collection should have
   // failed before.
   TRI_ASSERT(c != nullptr);
-  std::unique_ptr<LogicalCollection> newCol(c->clone());
-  return newCol.release();
+  return c->clone();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -1079,11 +1076,11 @@ static void CreateVocBase(v8::FunctionCallbackInfo<v8::Value> const& args,
 #ifndef USE_ENTERPRISE
     auto parameters = std::make_unique<LogicalCollection>(vocbase, infoSlice, false);
     TRI_V8_RETURN(
-        WrapCollection(isolate, CreateCollectionCoordinator(parameters.get())));
+        WrapCollection(isolate, CreateCollectionCoordinator(parameters.get()).release()));
 #else
     TRI_V8_RETURN(
         WrapCollection(isolate, CreateCollectionCoordinatorEnterprise(
-                                    collectionType, vocbase, infoSlice)));
+                                    collectionType, vocbase, infoSlice).release()));
 #endif
   }
 

arangod/V8Server/v8-vocindex.h

@@ -50,11 +50,13 @@ void TRI_InitV8indexCollection(v8::Isolate* isolate,
                                v8::Handle<v8::ObjectTemplate> rt);
 
 // This could be static but is used in enterprise version as well
-arangodb::LogicalCollection* CreateCollectionCoordinator(
+// Note that this returns a newly allocated object and ownership is transferred
+// to the caller, which is expressed by the returned unique_ptr.
+std::unique_ptr<arangodb::LogicalCollection> CreateCollectionCoordinator(
     arangodb::LogicalCollection* parameters);
 
 #ifdef USE_ENTERPRISE
-arangodb::LogicalCollection* CreateCollectionCoordinatorEnterprise(
+std::unique_ptr<arangodb::LogicalCollection> CreateCollectionCoordinatorEnterprise(
     TRI_col_type_e collectionType, TRI_vocbase_t* vocbase,
     arangodb::velocypack::Slice parameters);
 #endif

arangod/VocBase/LogicalCollection.h

@@ -68,17 +68,19 @@ class LogicalCollection {
  public:
   LogicalCollection(TRI_vocbase_t*, arangodb::velocypack::Slice const&, bool isPhysical);
 
-  explicit LogicalCollection(LogicalCollection const&);
-
   virtual ~LogicalCollection();
 
+ protected:  // If you need a copy outside the class, use clone below.
+  explicit LogicalCollection(LogicalCollection const&);
+
  private:
   LogicalCollection& operator=(LogicalCollection const&) = delete;
  public:
   LogicalCollection() = delete;
   
-  virtual LogicalCollection* clone() {
-    return new LogicalCollection(*this);
+  virtual std::unique_ptr<LogicalCollection> clone() {
+    auto p = new LogicalCollection(*this);
+    return std::unique_ptr<LogicalCollection>(p);
   }
 
   /// @brief hard-coded minimum version number for collections