SERVER-8170: cleanup and unify GC callbacks

11 files changed, 56 insertions, 67 deletions

diff --git a/src/mongo/db/commands/group.cpp b/src/mongo/db/commands/group.cpp
index bd7b439620a..1750f3e5238 100644
--- a/src/mongo/db/commands/group.cpp
+++ b/src/mongo/db/commands/group.cpp
@@ -53,9 +53,9 @@ namespace mongo {
                 const BSONObj& key = b.obj();
                 int res = s->invoke( func , &key, 0 );
                 uassert( 10041 ,  (string)"invoke failed in $keyf: " + s->getError() , res == 0 );
-                int type = s->type("return");
+                int type = s->type("__returnValue");
                 uassert( 10042 ,  "return of $key has to be an object" , type == Object );
-                return s->getObject( "return" );
+                return s->getObject( "__returnValue" );
             }
             return obj.extractFields( keyPattern , true ).getOwned();
         }
diff --git a/src/mongo/db/commands/mr.cpp b/src/mongo/db/commands/mr.cpp
index e775b4d083b..047071534fe 100644
--- a/src/mongo/db/commands/mr.cpp
+++ b/src/mongo/db/commands/mr.cpp
@@ -91,7 +91,7 @@ namespace mongo {
             // is converting many fields to 1
             BSONObjBuilder b;
             b.append( o.firstElement() );
-            s->append( b , "value" , "return" );
+            s->append( b , "value" , "__returnValue" );
             return b.obj();
         }
 
@@ -111,7 +111,7 @@ namespace mongo {
 
             BSONObjBuilder b(endSizeEstimate);
             b.appendAs( key.firstElement() , "0" );
-            _func.scope()->append( b , "1" , "return" );
+            _func.scope()->append( b , "1" , "__returnValue" );
             return b.obj();
         }
 
@@ -139,7 +139,7 @@ namespace mongo {
                 _reduce( tuples , key , endSizeEstimate );
                 BSONObjBuilder b(endSizeEstimate);
                 b.appendAs( key.firstElement() , "_id" );
-                _func.scope()->append( b , "value" , "return" );
+                _func.scope()->append( b , "value" , "__returnValue" );
                 res = b.obj();
             }
 
@@ -195,7 +195,7 @@ namespace mongo {
             s->invokeSafe( _func.func() , &args, 0, 0, false, true, true );
             ++numReduces;
 
-            if ( s->type( "return" ) == Array ) {
+            if ( s->type( "__returnValue" ) == Array ) {
                 uasserted( 10075 , "reduce -> multiple not supported yet");
                 return;
             }
@@ -213,7 +213,7 @@ namespace mongo {
             }
             BSONObjBuilder temp( endSizeEstimate );
             temp.append( key.firstElement() );
-            s->append( temp , "1" , "return" );
+            s->append( temp , "1" , "__returnValue" );
             x.push_back( temp.obj() );
             _reduce( x , key , endSizeEstimate );
         }
@@ -393,7 +393,7 @@ namespace mongo {
             if (_jsMode) {
                 ScriptingFunction getResult = _scope->createFunction("var map = _mrMap; var result = []; for (key in map) { result.push({_id: key, value: map[key]}) } return result;");
                 _scope->invoke(getResult, 0, 0, 0, false);
-                BSONObj obj = _scope->getObject("return");
+                BSONObj obj = _scope->getObject("__returnValue");
                 final.append("results", BSONArray(obj));
                 return;
             }
diff --git a/src/mongo/db/commands/mr.h b/src/mongo/db/commands/mr.h
index 84006314b10..526aad3043a 100644
--- a/src/mongo/db/commands/mr.h
+++ b/src/mongo/db/commands/mr.h
@@ -120,7 +120,7 @@ namespace mongo {
         private:
 
             /**
-             * result in "return"
+             * result in "__returnValue"
              * @param key OUT
              * @param endSizeEstimate OUT
             */
diff --git a/src/mongo/db/dbeval.cpp b/src/mongo/db/dbeval.cpp
index 4defe2492e3..1b75fabe1a1 100644
--- a/src/mongo/db/dbeval.cpp
+++ b/src/mongo/db/dbeval.cpp
@@ -101,7 +101,7 @@ namespace mongo {
             return false;
         }
 
-        s->append( result , "retval" , "return" );
+        s->append( result , "retval" , "__returnValue" );
 
         return true;
     }
diff --git a/src/mongo/db/matcher.cpp b/src/mongo/db/matcher.cpp
index 13336251c40..3b4a408ab67 100644
--- a/src/mongo/db/matcher.cpp
+++ b/src/mongo/db/matcher.cpp
@@ -112,7 +112,7 @@ namespace mongo {
                 uassert( 10072 , "unknown error in invocation of $where function", false);
             }
             
-            return _scope->getBoolean( "return" ) != 0;
+            return _scope->getBoolean( "__returnValue" ) != 0;
         }
         
     private:
@@ -1166,7 +1166,7 @@ namespace mongo {
                 u_assert( 10072 , "unknown error in invocation of $where function", false);
                 return false;
             }
-            return _where->scope->getBoolean( "return" ) != 0;
+            return _where->scope->getBoolean( "__returnValue" ) != 0;
 
         }
     }
diff --git a/src/mongo/dbtests/jstests.cpp b/src/mongo/dbtests/jstests.cpp
index d317ce13991..02e89962f98 100644
--- a/src/mongo/dbtests/jstests.cpp
+++ b/src/mongo/dbtests/jstests.cpp
@@ -111,22 +111,22 @@ namespace JSTests {
             ASSERT( 5 == s->getNumber( "x" ) );
 
             s->invoke( "return 17;" , 0, 0 );
-            ASSERT( 17 == s->getNumber( "return" ) );
+            ASSERT( 17 == s->getNumber( "__returnValue" ) );
 
             s->invoke( "function(){ return 17; }" , 0, 0 );
-            ASSERT( 17 == s->getNumber( "return" ) );
+            ASSERT( 17 == s->getNumber( "__returnValue" ) );
 
             s->setNumber( "x" , 1.76 );
             s->invoke( "return x == 1.76; " , 0, 0 );
-            ASSERT( s->getBoolean( "return" ) );
+            ASSERT( s->getBoolean( "__returnValue" ) );
 
             s->setNumber( "x" , 1.76 );
             s->invoke( "return x == 1.79; " , 0, 0 );
-            ASSERT( ! s->getBoolean( "return" ) );
+            ASSERT( ! s->getBoolean( "__returnValue" ) );
 
             BSONObj obj = BSON( "" << 11.0 );
             s->invoke( "function( z ){ return 5 + z; }" , &obj, 0 );
-            ASSERT_EQUALS( 16 , s->getNumber( "return" ) );
+            ASSERT_EQUALS( 16 , s->getNumber( "__returnValue" ) );
 
             delete s;
         }
@@ -210,45 +210,45 @@ namespace JSTests {
             s->setObject( "blah" , o );
 
             s->invoke( "return blah.x;" , 0, 0 );
-            ASSERT_EQUALS( 17 , s->getNumber( "return" ) );
+            ASSERT_EQUALS( 17 , s->getNumber( "__returnValue" ) );
             s->invoke( "return blah.y;" , 0, 0 );
-            ASSERT_EQUALS( "eliot" , s->getString( "return" ) );
+            ASSERT_EQUALS( "eliot" , s->getString( "__returnValue" ) );
 
             s->invoke( "return this.z;" , 0, &o );
-            ASSERT_EQUALS( "sara" , s->getString( "return" ) );
+            ASSERT_EQUALS( "sara" , s->getString( "__returnValue" ) );
 
             s->invoke( "return this.z == 'sara';" , 0, &o );
-            ASSERT_EQUALS( true , s->getBoolean( "return" ) );
+            ASSERT_EQUALS( true , s->getBoolean( "__returnValue" ) );
 
             s->invoke( "this.z == 'sara';" , 0, &o );
-            ASSERT_EQUALS( true , s->getBoolean( "return" ) );
+            ASSERT_EQUALS( true , s->getBoolean( "__returnValue" ) );
 
             s->invoke( "this.z == 'asara';" , 0, &o );
-            ASSERT_EQUALS( false , s->getBoolean( "return" ) );
+            ASSERT_EQUALS( false , s->getBoolean( "__returnValue" ) );
 
             s->invoke( "return this.x == 17;" , 0, &o );
-            ASSERT_EQUALS( true , s->getBoolean( "return" ) );
+            ASSERT_EQUALS( true , s->getBoolean( "__returnValue" ) );
 
             s->invoke( "return this.x == 18;" , 0, &o );
-            ASSERT_EQUALS( false , s->getBoolean( "return" ) );
+            ASSERT_EQUALS( false , s->getBoolean( "__returnValue" ) );
 
             s->invoke( "function(){ return this.x == 17; }" , 0, &o );
-            ASSERT_EQUALS( true , s->getBoolean( "return" ) );
+            ASSERT_EQUALS( true , s->getBoolean( "__returnValue" ) );
 
             s->invoke( "function(){ return this.x == 18; }" , 0, &o );
-            ASSERT_EQUALS( false , s->getBoolean( "return" ) );
+            ASSERT_EQUALS( false , s->getBoolean( "__returnValue" ) );
 
             s->invoke( "function (){ return this.x == 17; }" , 0, &o );
-            ASSERT_EQUALS( true , s->getBoolean( "return" ) );
+            ASSERT_EQUALS( true , s->getBoolean( "__returnValue" ) );
 
             s->invoke( "function z(){ return this.x == 18; }" , 0, &o );
-            ASSERT_EQUALS( false , s->getBoolean( "return" ) );
+            ASSERT_EQUALS( false , s->getBoolean( "__returnValue" ) );
 
             s->invoke( "function (){ this.x == 17; }" , 0, &o );
-            ASSERT_EQUALS( false , s->getBoolean( "return" ) );
+            ASSERT_EQUALS( false , s->getBoolean( "__returnValue" ) );
 
             s->invoke( "function z(){ this.x == 18; }" , 0, &o );
-            ASSERT_EQUALS( false , s->getBoolean( "return" ) );
+            ASSERT_EQUALS( false , s->getBoolean( "__returnValue" ) );
 
             s->invoke( "x = 5; for( ; x <10; x++){ a = 1; }" , 0, &o );
             ASSERT_EQUALS( 10 , s->getNumber( "x" ) );
@@ -399,7 +399,7 @@ namespace JSTests {
                 s->setObject( "x" , o );
 
                 s->invoke( "return x.d.getTime() != 12;" , 0, 0 );
-                ASSERT_EQUALS( true, s->getBoolean( "return" ) );
+                ASSERT_EQUALS( true, s->getBoolean( "__returnValue" ) );
 
                 s->invoke( "z = x.d.getTime();" , 0, 0 );
                 ASSERT_EQUALS( 123456789 , s->getNumber( "z" ) );
@@ -507,7 +507,7 @@ namespace JSTests {
 
             s->setObject( "z" , o );
             s->invoke( "return z" , 0, 0 );
-            BSONObj out = s->getObject( "return" );
+            BSONObj out = s->getObject( "__returnValue" );
             ASSERT_EQUALS( 5 , out["a"].number() );
             ASSERT_EQUALS( 5.6 , out["b"].number() );
 
@@ -525,7 +525,7 @@ namespace JSTests {
 
             s->setObject( "z" , o , false );
             s->invoke( "return z" , 0, 0 );
-            out = s->getObject( "return" );
+            out = s->getObject( "__returnValue" );
             ASSERT_EQUALS( 5 , out["a"].number() );
             ASSERT_EQUALS( 5.6 , out["b"].number() );
 
@@ -575,7 +575,7 @@ namespace JSTests {
 //
 //            s->setObject( "z" , o , false );
 //            s->invoke( "return z" , BSONObj() );
-//            out = s->getObject( "return" );
+//            out = s->getObject( "__returnValue" );
 //            ASSERT_EQUALS( 3 , out["a"].number() );
 //            ASSERT_EQUALS( 4.5 , out["b"].number() );
 //
@@ -1091,7 +1091,7 @@ namespace JSTests {
             double n = 0;
             for ( ; n < 100000; n++ ) {
                 s->invoke( f , &empty, &start );
-                ASSERT_EQUALS( 11 , s->getNumber( "return" ) );
+                ASSERT_EQUALS( 11 , s->getNumber( "__returnValue" ) );
             }
             //cout << "speed1: " << ( n / t.millis() ) << " ops/ms" << endl;
         }
diff --git a/src/mongo/scripting/engine_spidermonkey.cpp b/src/mongo/scripting/engine_spidermonkey.cpp
index 69d7630b2d7..e920d44243b 100644
--- a/src/mongo/scripting/engine_spidermonkey.cpp
+++ b/src/mongo/scripting/engine_spidermonkey.cpp
@@ -1715,7 +1715,7 @@ namespace spidermonkey {
             }
 
             if ( ! ignoreReturn ) {
-                verify( JS_SetProperty( _context , _global , "return" , &rval ) );
+                verify( JS_SetProperty( _context , _global , "__returnValue" , &rval ) );
             }
 
             return 0;
diff --git a/src/mongo/scripting/engine_v8.cpp b/src/mongo/scripting/engine_v8.cpp
index b5e8908ba30..ab15eccb946 100644
--- a/src/mongo/scripting/engine_v8.cpp
+++ b/src/mongo/scripting/engine_v8.cpp
@@ -52,22 +52,11 @@ namespace mongo {
       return (BSONHolder*)ptr;
     }
 
-    static void weakRefBSONCallback(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));
-        BSONHolder* data = (BSONHolder*) field->Value();
-        delete data;
-        p.Dispose();
-    }
-
     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, weakRefBSONCallback);
+        p.MakeWeak(data, deleteOnCollect<BSONHolder>);
         return p;
     }
 
@@ -943,7 +932,7 @@ namespace mongo {
             else {
                 _lastRetIsNativeCode = false;
             }
-            _global->ForceSet(v8::String::New("return"), result);
+            _global->ForceSet(v8::String::New("__returnValue"), result);
         }
 
         return 0;
@@ -1709,7 +1698,7 @@ namespace mongo {
                                    v8::Handle<v8::Value> value, int depth,
                                    BSONObj* originalParent) {
         if (value->IsString()) {
-            b.append(sname, toSTLString(value).c_str());
+            b.append(sname, toSTLString(value));
             return;
         }
         if (value->IsFunction()) {
diff --git a/src/mongo/scripting/engine_v8.h b/src/mongo/scripting/engine_v8.h
index 6d3deff7ada..40f4c40e64b 100644
--- a/src/mongo/scripting/engine_v8.h
+++ b/src/mongo/scripting/engine_v8.h
@@ -42,6 +42,17 @@ namespace mongo {
 
     typedef v8::Handle<v8::Value> (*v8Function)(V8Scope* scope, const v8::Arguments& args);
 
+    /**
+     * v8 callback for persistent handles that are to be freed by the GC
+     */
+    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 {
     public:
         BSONHolder(BSONObj obj) {
diff --git a/src/mongo/scripting/v8_db.cpp b/src/mongo/scripting/v8_db.cpp
index 6e7866515af..d0733c2d7ca 100644
--- a/src/mongo/scripting/v8_db.cpp
+++ b/src/mongo/scripting/v8_db.cpp
@@ -103,11 +103,6 @@ namespace mongo {
         return mongo;
     }
 
-    void destroyConnection(v8::Persistent<v8::Value> self, void* parameter) {
-        delete static_cast<DBClientBase*>(parameter);
-        self.Dispose();
-        self.Clear();
-    }
 
     v8::Handle<v8::Value> mongoConsExternal(V8Scope* scope, const v8::Arguments& args) {
         char host[255];
@@ -132,7 +127,7 @@ namespace mongo {
         }
 
         v8::Persistent<v8::Object> self = v8::Persistent<v8::Object>::New(args.Holder());
-        self.MakeWeak(conn, destroyConnection);
+        self.MakeWeak(conn, deleteOnCollect<DBClientWithCommands>);
 
         ScriptEngine::runConnectCallback(*conn);
 
@@ -148,7 +143,7 @@ namespace mongo {
 
         DBClientBase* conn = createDirectClient();
         v8::Persistent<v8::Object> self = v8::Persistent<v8::Object>::New(args.This());
-        self.MakeWeak(conn, destroyConnection);
+        self.MakeWeak(conn, deleteOnCollect<DBClientBase>);
 
         args.This()->SetInternalField(0, v8::External::New(conn));
         args.This()->ForceSet(scope->v8StringData("slaveOk"), v8::Boolean::New(false));
@@ -164,12 +159,6 @@ namespace mongo {
         return conn;
     }
 
-    void destroyCursor(v8::Persistent<v8::Value> self, void* parameter) {
-        delete static_cast<mongo::DBClientCursor*>(parameter);
-        self.Dispose();
-        self.Clear();
-    }
-
     /**
      * JavaScript binding for Mongo.prototype.find(namespace, query, fields, limit, skip)
      */
@@ -204,8 +193,8 @@ namespace mongo {
         }
 
         v8::Persistent<v8::Object> c = v8::Persistent<v8::Object>::New(cons->NewInstance());
-        c.MakeWeak(cursor.get(), destroyCursor);
-        c->SetInternalField(0, v8::External::New(cursor.release()));
+        c->SetInternalField(0, v8::External::New(cursor.get()));
+        c.MakeWeak(cursor.release(), deleteOnCollect<DBClientCursor>);
         return c;
     }
 
diff --git a/src/mongo/scripting/v8_utils.cpp b/src/mongo/scripting/v8_utils.cpp
index dcc6c89ec9e..e083cd58df4 100644
--- a/src/mongo/scripting/v8_utils.cpp
+++ b/src/mongo/scripting/v8_utils.cpp
@@ -42,7 +42,7 @@ namespace mongo {
     }
 
     /** Get the properties of an object (and it's prototype) as a comma-delimited string */
-    std::string v8objectToString(const v8::Handle<v8::Object>& o) {
+    std::string v8ObjectToString(const v8::Handle<v8::Object>& o) {
         v8::Local<v8::Array> properties = o->GetPropertyNames();
         v8::String::Utf8Value str(properties);
         massert(16696 , "error converting js type to Utf8Value", *str);