SERVER-8170: manually free external resources upon V8Scope destruction

3 files changed, 57 insertions, 68 deletions

diff --git a/src/mongo/scripting/engine_v8.cpp b/src/mongo/scripting/engine_v8.cpp
index 9179626e6e8..2cd074fdc1a 100644
--- a/src/mongo/scripting/engine_v8.cpp
+++ b/src/mongo/scripting/engine_v8.cpp
@@ -52,29 +52,17 @@ namespace mongo {
       return (BSONHolder*)ptr;
     }
 
-    v8::Persistent<v8::Object> V8Scope::wrapBSONObject(v8::Local<v8::Object> obj,
-                                                       BSONHolder* data) {
-        obj->SetInternalField(0, v8::External::New(data));
-        v8::Persistent<v8::Object> p = v8::Persistent<v8::Object>::New(obj);
-        p.MakeWeak(data, deleteOnCollect<BSONHolder>);
-        return p;
-    }
-
-    static void weakRefArrayCallback(v8::Persistent<v8::Value> p, void* scope) {
-        v8::HandleScope handle_scope;
-        if (!p.IsNearDeath())
-            return;
-        v8::Handle<v8::External> field =
-                v8::Handle<v8::External>::Cast(p->ToObject()->GetInternalField(0));
-        char* data = (char*) field->Value();
-        delete [] data;
-        p.Dispose();
+    void deleteBSONHolderOnCollect(v8::Persistent<v8::Value> objHandle, void* obj) {
+        BSONHolder* holder = static_cast<BSONHolder*>(obj);
+        holder->_scope->_lazyObjects.erase(holder);
+        deleteOnCollect<BSONHolder>(objHandle, obj);
     }
 
-    v8::Persistent<v8::Object> V8Scope::wrapArrayObject(v8::Local<v8::Object> obj, char* data) {
+    v8::Persistent<v8::Object> V8Scope::wrapBSONObject(v8::Local<v8::Object> obj,
+                                                       BSONHolder* data) {
         obj->SetInternalField(0, v8::External::New(data));
         v8::Persistent<v8::Object> p = v8::Persistent<v8::Object>::New(obj);
-        p.MakeWeak(this, weakRefArrayCallback);
+        p.MakeWeak(data, deleteBSONHolderOnCollect);
         return p;
     }
 
@@ -572,8 +560,21 @@ namespace mongo {
             roObjectTemplate.Dispose();
             internalFieldObjects.Dispose();
             _context.Dispose();
+            gc(); // collect any known garbage after _global and _context have been disposed
         }
         _isolate->Dispose();
+        // NOTE: Persistent external objects may not be collected after destroying the global
+        //       object and context.  Destroying the isolate frees its heap, but does not
+        //       execute the callbacks supplied to v8::Persistent::MakeWeak().
+        // free any remaining BSONHolder instances
+        if (!_lazyObjects.empty())
+            LOG(1) << "freeing uncollected BSONHolders: " << _lazyObjects.size() << endl;
+        for (set<BSONHolder*>::iterator it = _lazyObjects.begin();
+             it != _lazyObjects.end();
+             ++it) {
+            (*it)->_notifyV8Memory = false;
+            delete *it;
+        }
     }
 
     bool V8Scope::hasOutOfMemoryException() {
@@ -1447,6 +1448,9 @@ namespace mongo {
     v8::Persistent<v8::Object> V8Scope::mongoToLZV8(const BSONObj& m, bool readOnly) {
         v8::Local<v8::Object> o;
         BSONHolder* own = new BSONHolder(m);
+        // track the BSONHolder so the object can be freed when the V8Scope is destructed
+        own->_scope = this;
+        _lazyObjects.insert(own);
 
         if (readOnly) {
             o = roObjectTemplate->NewInstance();
@@ -1691,12 +1695,10 @@ namespace mongo {
                                    const string& elementName,
                                    v8::Handle<v8::Object> obj) {
         int len = obj->Get(v8StringData("len"))->ToInt32()->Value();
-        v8::Local<v8::External> c = v8::External::Cast(*(obj->GetInternalField(0)));
-        const char* dataArray = static_cast <const char*>(c->Value());
         b.appendBinData(elementName,
                         len,
                         mongo::BinDataType(obj->Get(v8StringData("type"))->ToInt32()->Value()),
-                        dataArray);
+                        base64::decode(toSTLString(obj->GetInternalField(0))).c_str());
     }
 
     void V8Scope::v8ToMongoObjectID(BSONObjBuilder& b,
diff --git a/src/mongo/scripting/engine_v8.h b/src/mongo/scripting/engine_v8.h
index bd31af737f5..5d14f4cfa31 100644
--- a/src/mongo/scripting/engine_v8.h
+++ b/src/mongo/scripting/engine_v8.h
@@ -20,6 +20,7 @@
 #include <v8.h>
 #include <vector>
 
+#include "mongo/base/disallow_copying.h"
 #include "mongo/scripting/engine.h"
 #include "mongo/scripting/v8_deadline_monitor.h"
 #include "mongo/scripting/v8_profiler.h"
@@ -47,24 +48,29 @@ namespace mongo {
      */
     template <typename _T>
     void deleteOnCollect(v8::Persistent<v8::Value> objHandle, void* obj) {
-        if (!objHandle.IsNearDeath())
-            return;
         delete static_cast<_T*>(obj);
         objHandle.Dispose();
     }
 
     class BSONHolder {
+    MONGO_DISALLOW_COPYING(BSONHolder);
     public:
-        BSONHolder(BSONObj obj) {
-            _obj = obj.getOwned();
-            _modified = false;
+        explicit BSONHolder(BSONObj obj) :
+            _obj(obj.getOwned()),
+            _modified(false),
+            _notifyV8Memory(true) {
+            // give hint v8's GC
             v8::V8::AdjustAmountOfExternalAllocatedMemory(_obj.objsize());
         }
         ~BSONHolder() {
-            v8::V8::AdjustAmountOfExternalAllocatedMemory(-_obj.objsize());
+            if (_notifyV8Memory)
+                // if v8 is still up, send hint to GC
+                v8::V8::AdjustAmountOfExternalAllocatedMemory(-_obj.objsize());
         }
+        V8Scope* _scope;
         BSONObj _obj;
         bool _modified;
+        bool _notifyV8Memory; // set to true if v8 is still usable during destruction
         list<string> _extra;
         set<string> _removed;
     };
@@ -228,7 +234,6 @@ namespace mongo {
          * GC callback for weak references to BSON objects (via BSONHolder)
          */
         v8::Persistent<v8::Object> wrapBSONObject(v8::Local<v8::Object> obj, BSONHolder* data);
-        v8::Persistent<v8::Object> wrapArrayObject(v8::Local<v8::Object> obj, char* data);
 
         /**
          * Create a V8 string with a local handle
@@ -248,6 +253,9 @@ namespace mongo {
          */
         v8::Persistent<v8::Context> getContext() { return _context; }
 
+        // store a pointer to all BSONHolder instances created in this V8Scope
+        set<BSONHolder*> _lazyObjects;
+
     private:
 
         /**
diff --git a/src/mongo/scripting/v8_db.cpp b/src/mongo/scripting/v8_db.cpp
index 181d0d5db8c..dbc12685db5 100644
--- a/src/mongo/scripting/v8_db.cpp
+++ b/src/mongo/scripting/v8_db.cpp
@@ -638,65 +638,43 @@ namespace mongo {
         }
 
         v8::Handle<v8::Value> type;
-        v8::Handle<v8::Value> len;
-        int rlen;
-        char* data;
         if (args.Length() == 2) {
             // 2 args: type, base64 string
             type = args[0];
             v8::String::Utf8Value utf(args[1]);
-            string decoded = base64::decode(*utf);
-            const char* tmp = decoded.data();
-            rlen = decoded.length();
-            data = new char[rlen];
-            memcpy(data, tmp, rlen);
-            len = v8::Number::New(rlen);
-        }
-        else if (args.Length() == 0) {
-            // this is called by subclasses that will fill properties
-            return it;
-        }
-        else {
+            // uassert if invalid base64 string
+            string tmpBase64 = base64::decode(*utf);
+            // length property stores the decoded length
+            it->ForceSet(scope->v8StringData("len"), v8::Number::New(tmpBase64.length()));
+            it->ForceSet(scope->v8StringData("type"), type);
+            it->SetHiddenValue(v8::String::New("__BinData"), v8::Number::New(1));
+            it->SetInternalField(0, args[1]);
+        }
+        else if (args.Length() != 0) {
             return v8AssertionException("BinData takes 2 arguments -- BinData(subtype,data)");
         }
 
-        it->ForceSet(scope->v8StringData("len"), len);
-        it->ForceSet(scope->v8StringData("type"), type);
-        it->SetHiddenValue(v8::String::New("__BinData"), v8::Number::New(1));
-        v8::Persistent<v8::Object> res = scope->wrapArrayObject(it, data);
-        return res;
+        return it;
     }
 
     v8::Handle<v8::Value> binDataToString(V8Scope* scope, const v8::Arguments& args) {
         v8::Handle<v8::Object> it = args.This();
-        int len = it->Get(v8::String::New("len"))->Int32Value();
         int type = it->Get(v8::String::New("type"))->Int32Value();
-        v8::Local<v8::External> c = v8::External::Cast(*(it->GetInternalField(0)));
-        char* data = (char*)(c->Value());
 
         stringstream ss;
-        ss << "BinData(" << type << ",\"";
-        base64::encode(ss, data, len);
-        ss << "\")";
-        string ret = ss.str();
-        return v8::String::New(ret.c_str());
+        ss << "BinData(" << type << ",\"" << toSTLString(it->GetInternalField(0)) << "\")";
+        return v8::String::New(ss.str().c_str());
     }
 
     v8::Handle<v8::Value> binDataToBase64(V8Scope* scope, const v8::Arguments& args) {
         v8::Handle<v8::Object> it = args.This();
-        int len = v8::Handle<v8::Number>::Cast(it->Get(v8::String::New("len")))->Int32Value();
-        v8::Local<v8::External> c = v8::External::Cast(*(it->GetInternalField(0)));
-        char* data = (char*)(c->Value());
-        stringstream ss;
-        base64::encode(ss, (const char*)data, len);
-        return v8::String::New(ss.str().c_str());
+        return it->GetInternalField(0);
     }
 
     v8::Handle<v8::Value> binDataToHex(V8Scope* scope, const v8::Arguments& args) {
         v8::Handle<v8::Object> it = args.This();
         int len = v8::Handle<v8::Number>::Cast(it->Get(v8::String::New("len")))->Int32Value();
-        v8::Local<v8::External> c = v8::External::Cast(*(it->GetInternalField(0)));
-        char* data = (char*)(c->Value());
+        string data = base64::decode(toSTLString(it->GetInternalField(0)));
         stringstream ss;
         ss.setf (ios_base::hex, ios_base::basefield);
         ss.fill ('0');
@@ -711,17 +689,18 @@ namespace mongo {
     static v8::Handle<v8::Value> hexToBinData(V8Scope* scope, v8::Local<v8::Object> it, int type,
                                               string hexstr) {
         int len = hexstr.length() / 2;
-        char* data = new char[len];
+        scoped_array<char> data(new char[16]);
         const char* src = hexstr.c_str();
         for(int i = 0; i < 16; i++) {
             data[i] = fromHex(src + i * 2);
         }
 
+        string encoded = base64::encode(data.get(), 16);
         it->ForceSet(v8::String::New("len"), v8::Number::New(len));
         it->ForceSet(v8::String::New("type"), v8::Number::New(type));
         it->SetHiddenValue(v8::String::New("__BinData"), v8::Number::New(1));
-        v8::Persistent<v8::Object> res = scope->wrapArrayObject(it, data);
-        return res;
+        it->SetInternalField(0, v8::String::New(encoded.c_str(), encoded.length()));
+        return it;
     }
 
     v8::Handle<v8::Value> uuidInit(V8Scope* scope, const v8::Arguments& args) {