diff options
author | Jason Carey <jcarey@argv.me> | 2019-01-03 16:38:26 -0500 |
---|---|---|
committer | Jason Carey <jcarey@argv.me> | 2019-02-10 12:23:19 -0500 |
commit | 21b5477520744ca02e1574160caa31e0633849dd (patch) | |
tree | 36f351a91a195a633c9cc495f6249057497249ff | |
parent | c922cb18516981ceca59993331296d102f4e01fb (diff) | |
download | mongo-21b5477520744ca02e1574160caa31e0633849dd.tar.gz |
SERVER-39183 honor socket disconnect in $where
-rw-r--r-- | jstests/noPassthrough/socket_disconnect_kills.js | 23 | ||||
-rw-r--r-- | src/mongo/db/commands/mr.cpp | 1 | ||||
-rw-r--r-- | src/mongo/db/matcher/expression_where.cpp | 6 | ||||
-rw-r--r-- | src/mongo/dbtests/jstests.cpp | 244 | ||||
-rw-r--r-- | src/mongo/scripting/engine.h | 14 | ||||
-rw-r--r-- | src/mongo/scripting/mozjs/implscope.cpp | 18 | ||||
-rw-r--r-- | src/mongo/scripting/mozjs/implscope.h | 2 | ||||
-rw-r--r-- | src/mongo/scripting/mozjs/proxyscope.cpp | 52 | ||||
-rw-r--r-- | src/mongo/scripting/mozjs/proxyscope.h | 14 |
9 files changed, 237 insertions, 137 deletions
diff --git a/jstests/noPassthrough/socket_disconnect_kills.js b/jstests/noPassthrough/socket_disconnect_kills.js index 1062296ff29..a1ba2e26e60 100644 --- a/jstests/noPassthrough/socket_disconnect_kills.js +++ b/jstests/noPassthrough/socket_disconnect_kills.js @@ -134,6 +134,29 @@ [[checkClosedEarly, runCommand({find: "test", filter: {}})], [ checkClosedEarly, + runCommand({ + find: "test", + filter: { + $where: function() { + sleep(100000); + } + } + }) + ], + [ + checkClosedEarly, + runCommand({ + find: "test", + filter: { + $where: function() { + while (true) { + } + } + } + }) + ], + [ + checkClosedEarly, function(client) { client.forceReadMode("legacy"); assert(client.getDB(testName).test.findOne({})); diff --git a/src/mongo/db/commands/mr.cpp b/src/mongo/db/commands/mr.cpp index 450fe0c53dc..c9867cb05e9 100644 --- a/src/mongo/db/commands/mr.cpp +++ b/src/mongo/db/commands/mr.cpp @@ -253,7 +253,6 @@ void JSMapper::map(const BSONObj& o) { BSONObj JSFinalizer::finalize(const BSONObj& o) { Scope* s = _func.scope(); - Scope::NoDBAccess no = s->disableDBAccess("can't access db inside finalize"); s->invokeSafe(_func.func(), &o, 0); // We don't want to use o.objsize() to size b since there are many cases where the point of diff --git a/src/mongo/db/matcher/expression_where.cpp b/src/mongo/db/matcher/expression_where.cpp index c6633bc3802..7d1f261370f 100644 --- a/src/mongo/db/matcher/expression_where.cpp +++ b/src/mongo/db/matcher/expression_where.cpp @@ -43,6 +43,7 @@ #include "mongo/db/namespace_string.h" #include "mongo/scripting/engine.h" #include "mongo/stdx/memory.h" +#include "mongo/util/scopeguard.h" namespace mongo { @@ -67,6 +68,8 @@ WhereMatchExpression::WhereMatchExpression(OperationContext* opCtx, AuthorizationSession::get(Client::getCurrent())->getAuthenticatedUserNamesToken(); _scope = getGlobalScriptEngine()->getPooledScope(_opCtx, _dbName, "where" + userToken); + const auto guard = makeGuard([&] { _scope->unregisterOperation(); }); + _func = _scope->createFunction(getCode().c_str()); uassert(ErrorCodes::BadValue, "$where compile error", _func); @@ -76,6 +79,9 @@ bool WhereMatchExpression::matches(const MatchableDocument* doc, MatchDetails* d uassert(28692, "$where compile error", _func); BSONObj obj = doc->toBSON(); + _scope->registerOperation(Client::getCurrent()->getOperationContext()); + const auto guard = makeGuard([&] { _scope->unregisterOperation(); }); + if (!getScope().isEmpty()) { _scope->init(&getScope()); } diff --git a/src/mongo/dbtests/jstests.cpp b/src/mongo/dbtests/jstests.cpp index 9074a424367..cf606948c65 100644 --- a/src/mongo/dbtests/jstests.cpp +++ b/src/mongo/dbtests/jstests.cpp @@ -62,6 +62,9 @@ using std::vector; namespace JSTests { +using ScopeFactory = Scope* (ScriptEngine::*)(); + +template <ScopeFactory scopeFactory> class BuiltinTests { public: void run() { @@ -70,11 +73,12 @@ public: } }; +template <ScopeFactory scopeFactory> class BasicScope { public: void run() { unique_ptr<Scope> s; - s.reset(getGlobalScriptEngine()->newScope()); + s.reset((getGlobalScriptEngine()->*scopeFactory)()); s->setNumber("x", 5); ASSERT(5 == s->getNumber("x")); @@ -93,12 +97,13 @@ public: } }; +template <ScopeFactory scopeFactory> class ResetScope { public: void run() { /* Currently reset does not clear data in v8 or spidermonkey scopes. See SECURITY-10 unique_ptr<Scope> s; - s.reset( getGlobalScriptEngine()->newScope() ); + s.reset( (getGlobalScriptEngine()->*scopeFactory)() ); s->setBoolean( "x" , true ); ASSERT( s->getBoolean( "x" ) ); @@ -109,12 +114,13 @@ public: } }; +template <ScopeFactory scopeFactory> class FalseTests { public: void run() { // Test falsy javascript values unique_ptr<Scope> s; - s.reset(getGlobalScriptEngine()->newScope()); + s.reset((getGlobalScriptEngine()->*scopeFactory)()); ASSERT(!s->getBoolean("notSet")); @@ -133,10 +139,11 @@ public: } }; +template <ScopeFactory scopeFactory> class SimpleFunctions { public: void run() { - unique_ptr<Scope> s(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> s((getGlobalScriptEngine()->*scopeFactory)()); s->invoke("x=5;", 0, 0); ASSERT(5 == s->getNumber("x")); @@ -198,10 +205,11 @@ private: }; /** Error logging in Scope::exec(). */ +template <ScopeFactory scopeFactory> class ExecLogError { public: void run() { - unique_ptr<Scope> scope(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> scope((getGlobalScriptEngine()->*scopeFactory)()); // No error is logged when reportError == false. ASSERT(!scope->exec("notAFunction()", "foo", false, false, false)); @@ -229,10 +237,11 @@ private: }; /** Error logging in Scope::invoke(). */ +template <ScopeFactory scopeFactory> class InvokeLogError { public: void run() { - unique_ptr<Scope> scope(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> scope((getGlobalScriptEngine()->*scopeFactory)()); // No error is logged for a valid statement. ASSERT_EQUALS(0, scope->invoke("validStatement = true", 0, 0)); @@ -259,10 +268,11 @@ private: LogRecordingScope _logger; }; +template <ScopeFactory scopeFactory> class ObjectMapping { public: void run() { - unique_ptr<Scope> s(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> s((getGlobalScriptEngine()->*scopeFactory)()); BSONObj o = BSON("x" << 17.0 << "y" << "eliot" @@ -316,10 +326,11 @@ public: } }; +template <ScopeFactory scopeFactory> class ObjectDecoding { public: void run() { - unique_ptr<Scope> s(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> s((getGlobalScriptEngine()->*scopeFactory)()); s->invoke("z = { num : 1 };", 0, 0); BSONObj out = s->getObject("z"); @@ -338,11 +349,12 @@ public: } }; +template <ScopeFactory scopeFactory> class JSOIDTests { public: void run() { #ifdef MOZJS - unique_ptr<Scope> s(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> s((getGlobalScriptEngine()->*scopeFactory)()); s->localConnect("blah"); @@ -370,10 +382,11 @@ public: } }; +template <ScopeFactory scopeFactory> class SetImplicit { public: void run() { - unique_ptr<Scope> s(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> s((getGlobalScriptEngine()->*scopeFactory)()); BSONObj o = BSON("foo" << "bar"); @@ -392,10 +405,11 @@ public: } }; +template <ScopeFactory scopeFactory> class ObjectModReadonlyTests { public: void run() { - unique_ptr<Scope> s(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> s((getGlobalScriptEngine()->*scopeFactory)()); BSONObj o = BSON("x" << 17 << "y" << "eliot" @@ -430,10 +444,11 @@ public: } }; +template <ScopeFactory scopeFactory> class OtherJSTypes { public: void run() { - unique_ptr<Scope> s(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> s((getGlobalScriptEngine()->*scopeFactory)()); { // date @@ -527,10 +542,11 @@ public: } }; +template <ScopeFactory scopeFactory> class SpecialDBTypes { public: void run() { - unique_ptr<Scope> s(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> s((getGlobalScriptEngine()->*scopeFactory)()); BSONObjBuilder b; b.appendTimestamp("a", 123456789); @@ -561,10 +577,11 @@ public: } }; +template <ScopeFactory scopeFactory> class TypeConservation { public: void run() { - unique_ptr<Scope> s(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> s((getGlobalScriptEngine()->*scopeFactory)()); // -- A -- @@ -659,10 +676,11 @@ public: } }; +template <ScopeFactory scopeFactory> class NumberLong { public: void run() { - unique_ptr<Scope> s(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> s((getGlobalScriptEngine()->*scopeFactory)()); BSONObjBuilder b; long long val = (long long)(0xbabadeadbeefbaddULL); b.append("a", val); @@ -718,10 +736,11 @@ public: } }; +template <ScopeFactory scopeFactory> class NumberLong2 { public: void run() { - unique_ptr<Scope> s(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> s((getGlobalScriptEngine()->*scopeFactory)()); BSONObj in; { @@ -745,10 +764,11 @@ public: } }; +template <ScopeFactory scopeFactory> class NumberLongUnderLimit { public: void run() { - unique_ptr<Scope> s(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> s((getGlobalScriptEngine()->*scopeFactory)()); BSONObjBuilder b; // limit is 2^53 @@ -791,10 +811,11 @@ public: } }; +template <ScopeFactory scopeFactory> class NumberDecimal { public: void run() { - unique_ptr<Scope> s(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> s((getGlobalScriptEngine()->*scopeFactory)()); BSONObjBuilder b; Decimal128 val = Decimal128("2.010"); b.append("a", val); @@ -820,19 +841,21 @@ public: } }; +template <ScopeFactory scopeFactory> class NumberDecimalGetFromScope { public: void run() { - unique_ptr<Scope> s(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> s((getGlobalScriptEngine()->*scopeFactory)()); ASSERT(s->exec("a = 5;", "a", false, true, false)); ASSERT_TRUE(Decimal128(5).isEqual(s->getNumberDecimal("a"))); } }; +template <ScopeFactory scopeFactory> class NumberDecimalBigObject { public: void run() { - unique_ptr<Scope> s(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> s((getGlobalScriptEngine()->*scopeFactory)()); BSONObj in; { @@ -856,10 +879,11 @@ public: } }; +template <ScopeFactory scopeFactory> class MaxTimestamp { public: void run() { - unique_ptr<Scope> s(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> s((getGlobalScriptEngine()->*scopeFactory)()); // Timestamp 't' component can exceed max for int32_t. BSONObj in; @@ -877,6 +901,7 @@ public: } }; +template <ScopeFactory scopeFactory> class WeirdObjects { public: BSONObj build(int depth) { @@ -888,7 +913,7 @@ public: } void run() { - unique_ptr<Scope> s(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> s((getGlobalScriptEngine()->*scopeFactory)()); for (int i = 5; i < 100; i += 10) { s->setObject("a", build(i), false); @@ -903,10 +928,11 @@ public: /** * Test exec() timeout value terminates execution (SERVER-8053) */ +template <ScopeFactory scopeFactory> class ExecTimeout { public: void run() { - unique_ptr<Scope> scope(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> scope((getGlobalScriptEngine()->*scopeFactory)()); // assert timeout occurred ASSERT(!scope->exec("var a = 1; while (true) { ; }", "ExecTimeout", false, true, false, 1)); @@ -916,10 +942,11 @@ public: /** * Test exec() timeout value terminates execution (SERVER-8053) */ +template <ScopeFactory scopeFactory> class ExecNoTimeout { public: void run() { - unique_ptr<Scope> scope(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> scope((getGlobalScriptEngine()->*scopeFactory)()); // assert no timeout occurred ASSERT(scope->exec("var a = function() { return 1; }", @@ -934,10 +961,11 @@ public: /** * Test invoke() timeout value terminates execution (SERVER-8053) */ +template <ScopeFactory scopeFactory> class InvokeTimeout { public: void run() { - unique_ptr<Scope> scope(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> scope((getGlobalScriptEngine()->*scopeFactory)()); // scope timeout after 500ms bool caught = false; @@ -956,22 +984,18 @@ public: } }; +template <ScopeFactory scopeFactory> class SleepInterruption { public: void run() { - std::shared_ptr<Scope> scope(getGlobalScriptEngine()->newScope()); - - auto sleep = makePromiseFuture<void>(); - auto awakened = makePromiseFuture<void>(); + auto scopePF = makePromiseFuture<Scope*>(); + auto awakenedPF = makePromiseFuture<void>(); // Spawn a thread which attempts to sleep indefinitely. - stdx::thread([ - preSleep = std::move(sleep.promise), - onAwake = std::move(awakened.promise), - scope - ]() mutable { - preSleep.emplaceValue(); - onAwake.setWith([&] { + stdx::thread thread([&] { + std::unique_ptr<Scope> scope((getGlobalScriptEngine()->*scopeFactory)()); + scopePF.promise.emplaceValue(scope.get()); + awakenedPF.promise.setWith([&] { scope->exec( "" " try {" @@ -985,10 +1009,10 @@ public: false, true); }); - }).detach(); + }); // Wait until just before the sleep begins. - sleep.future.get(); + auto scope = scopePF.future.get(); // Attempt to wait until Javascript enters the sleep. // It's OK if we kill the function prematurely, before it begins sleeping. Either cause of @@ -999,18 +1023,21 @@ public: scope->kill(); // Wait for the error. - auto result = awakened.future.getNoThrow(); + auto result = awakenedPF.future.getNoThrow(); ASSERT_EQ(ErrorCodes::Interrupted, result); + + thread.join(); } }; /** * Test invoke() timeout value does not terminate execution (SERVER-8053) */ +template <ScopeFactory scopeFactory> class InvokeNoTimeout { public: void run() { - unique_ptr<Scope> scope(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> scope((getGlobalScriptEngine()->*scopeFactory)()); // invoke completes before timeout scope->invokeSafe( @@ -1024,6 +1051,7 @@ public: }; +template <ScopeFactory scopeFactory> class Utf8Check { public: Utf8Check() { @@ -1059,6 +1087,7 @@ private: }; +template <ScopeFactory scopeFactory> class BinDataType { public: void pp(const char* s, BSONElement e) { @@ -1072,7 +1101,7 @@ public: } void run() { - unique_ptr<Scope> s(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> s((getGlobalScriptEngine()->*scopeFactory)()); const char* foo = "asdas\0asdasd"; const char* base64 = "YXNkYXMAYXNkYXNk"; @@ -1119,10 +1148,11 @@ public: } }; +template <ScopeFactory scopeFactory> class VarTests { public: void run() { - unique_ptr<Scope> s(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> s((getGlobalScriptEngine()->*scopeFactory)()); ASSERT(s->exec("a = 5;", "a", false, true, false)); ASSERT_EQUALS(5, s->getNumber("a")); @@ -1132,6 +1162,7 @@ public: } }; +template <ScopeFactory scopeFactory> class Speed1 { public: void run() { @@ -1139,7 +1170,7 @@ public: BSONObj empty; unique_ptr<Scope> s; - s.reset(getGlobalScriptEngine()->newScope()); + s.reset((getGlobalScriptEngine()->*scopeFactory)()); ScriptingFunction f = s->createFunction("return this.x + 6;"); @@ -1153,11 +1184,12 @@ public: } }; +template <ScopeFactory scopeFactory> class ScopeOut { public: void run() { unique_ptr<Scope> s; - s.reset(getGlobalScriptEngine()->newScope()); + s.reset((getGlobalScriptEngine()->*scopeFactory)()); s->invokeSafe("x = 5;", 0, 0); { @@ -1179,11 +1211,12 @@ public: } }; +template <ScopeFactory scopeFactory> class RenameTest { public: void run() { unique_ptr<Scope> s; - s.reset(getGlobalScriptEngine()->newScope()); + s.reset((getGlobalScriptEngine()->*scopeFactory)()); s->setNumber("x", 5); ASSERT_EQUALS(5, s->getNumber("x")); @@ -1204,13 +1237,14 @@ public: * spidermonkey by looking like non-double type puns. This verifies that we put that particular * interesting nan in and that we still get a nan out. */ +template <ScopeFactory scopeFactory> class NovelNaN { public: void run() { uint8_t bits[] = { 16, 0, 0, 0, 0x01, 'a', '\0', 0x61, 0x79, 0xfe, 0xff, 0xff, 0xff, 0xff, 0xff, 0, }; - unique_ptr<Scope> s(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> s((getGlobalScriptEngine()->*scopeFactory)()); s->setObject("val", BSONObj(reinterpret_cast<char*>(bits)).getOwned()); @@ -1219,10 +1253,11 @@ public: } }; +template <ScopeFactory scopeFactory> class NoReturnSpecified { public: void run() { - unique_ptr<Scope> s(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> s((getGlobalScriptEngine()->*scopeFactory)()); s->invoke("x=5;", 0, 0); ASSERT_EQUALS(5, s->getNumber("__returnValue")); @@ -1265,6 +1300,7 @@ public: } }; +template <ScopeFactory scopeFactory> class RecursiveInvoke { public: static BSONObj callback(const BSONObj& args, void* data) { @@ -1276,7 +1312,7 @@ public: } void run() { - unique_ptr<Scope> s(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> s((getGlobalScriptEngine()->*scopeFactory)()); s->injectNative("foo", callback, s.get()); s->invoke("var x = 1; foo();", 0, 0); @@ -1284,10 +1320,11 @@ public: } }; +template <ScopeFactory scopeFactory> class ErrorCodeFromInvoke { public: void run() { - unique_ptr<Scope> s(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> s((getGlobalScriptEngine()->*scopeFactory)()); { bool threwException = false; @@ -1321,6 +1358,7 @@ public: } }; +template <ScopeFactory scopeFactory> class ErrorWithSidecarFromInvoke { public: void run() { @@ -1329,7 +1367,7 @@ public: return {}; }; - unique_ptr<Scope> s(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> s((getGlobalScriptEngine()->*scopeFactory)()); s->injectNative("foo", sidecarThrowingFunc); @@ -1340,6 +1378,7 @@ public: } }; +template <ScopeFactory scopeFactory> class RequiresOwnedObjects { public: void run() { @@ -1352,14 +1391,14 @@ public: // Ensure that by default we can bind owned and unowned { - unique_ptr<Scope> s(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> s((getGlobalScriptEngine()->*scopeFactory)()); s->setObject("unowned", unowned, true); s->setObject("owned", owned, true); } // After we set the flag, we should only be able to set owned { - unique_ptr<Scope> s(getGlobalScriptEngine()->newScope()); + unique_ptr<Scope> s((getGlobalScriptEngine()->*scopeFactory)()); s->requireOwnedObjects(); s->setObject("owned", owned, true); @@ -1383,6 +1422,7 @@ public: } }; +template <ScopeFactory scopeFactory> class ConvertShardKeyToHashed { public: void check(shared_ptr<Scope> s, const mongo::BSONObj& o) { @@ -1430,7 +1470,7 @@ public: } void run() { - shared_ptr<Scope> s(getGlobalScriptEngine()->newScope()); + shared_ptr<Scope> s((getGlobalScriptEngine()->*scopeFactory)()); shell_utils::installShellUtils(*s); // Check a few elementary objects @@ -1470,53 +1510,59 @@ class All : public Suite { public: All() : Suite("js") {} + template <ScopeFactory scopeFactory> + void setupTestsWithScopeFactory() { + add<BuiltinTests<scopeFactory>>(); + add<BasicScope<scopeFactory>>(); + add<ResetScope<scopeFactory>>(); + add<FalseTests<scopeFactory>>(); + add<SimpleFunctions<scopeFactory>>(); + add<ExecLogError<scopeFactory>>(); + add<InvokeLogError<scopeFactory>>(); + add<ExecTimeout<scopeFactory>>(); + add<ExecNoTimeout<scopeFactory>>(); + add<InvokeTimeout<scopeFactory>>(); + add<SleepInterruption<scopeFactory>>(); + add<InvokeNoTimeout<scopeFactory>>(); + + add<ObjectMapping<scopeFactory>>(); + add<ObjectDecoding<scopeFactory>>(); + add<JSOIDTests<scopeFactory>>(); + add<SetImplicit<scopeFactory>>(); + add<ObjectModReadonlyTests<scopeFactory>>(); + add<OtherJSTypes<scopeFactory>>(); + add<SpecialDBTypes<scopeFactory>>(); + add<TypeConservation<scopeFactory>>(); + add<NumberLong<scopeFactory>>(); + add<NumberLong2<scopeFactory>>(); + + add<NumberDecimal<scopeFactory>>(); + add<NumberDecimalGetFromScope<scopeFactory>>(); + add<NumberDecimalBigObject<scopeFactory>>(); + + add<MaxTimestamp<scopeFactory>>(); + add<RenameTest<scopeFactory>>(); + + add<WeirdObjects<scopeFactory>>(); + add<BinDataType<scopeFactory>>(); + + add<VarTests<scopeFactory>>(); + add<Speed1<scopeFactory>>(); + add<Utf8Check<scopeFactory>>(); + add<ScopeOut<scopeFactory>>(); + add<NovelNaN<scopeFactory>>(); + add<NoReturnSpecified<scopeFactory>>(); + + add<RecursiveInvoke<scopeFactory>>(); + add<ErrorCodeFromInvoke<scopeFactory>>(); + add<ErrorWithSidecarFromInvoke<scopeFactory>>(); + add<RequiresOwnedObjects<scopeFactory>>(); + add<ConvertShardKeyToHashed<scopeFactory>>(); + } + void setupTests() { - add<BuiltinTests>(); - add<BasicScope>(); - add<ResetScope>(); - add<FalseTests>(); - add<SimpleFunctions>(); - add<ExecLogError>(); - add<InvokeLogError>(); - add<ExecTimeout>(); - add<ExecNoTimeout>(); - add<InvokeTimeout>(); - add<SleepInterruption>(); - add<InvokeNoTimeout>(); - - add<ObjectMapping>(); - add<ObjectDecoding>(); - add<JSOIDTests>(); - add<SetImplicit>(); - add<ObjectModReadonlyTests>(); - add<OtherJSTypes>(); - add<SpecialDBTypes>(); - add<TypeConservation>(); - add<NumberLong>(); - add<NumberLong2>(); - - add<NumberDecimal>(); - add<NumberDecimalGetFromScope>(); - add<NumberDecimalBigObject>(); - - add<MaxTimestamp>(); - add<RenameTest>(); - - add<WeirdObjects>(); - add<BinDataType>(); - - add<VarTests>(); - add<Speed1>(); - add<Utf8Check>(); - add<ScopeOut>(); - add<NovelNaN>(); - add<NoReturnSpecified>(); - - add<RecursiveInvoke>(); - add<ErrorCodeFromInvoke>(); - add<ErrorWithSidecarFromInvoke>(); - add<RequiresOwnedObjects>(); - add<ConvertShardKeyToHashed>(); + setupTestsWithScopeFactory<&ScriptEngine::newScope>(); + setupTestsWithScopeFactory<&ScriptEngine::newScopeForCurrentThread>(); } }; diff --git a/src/mongo/scripting/engine.h b/src/mongo/scripting/engine.h index cfe130ec650..fef3d59aff5 100644 --- a/src/mongo/scripting/engine.h +++ b/src/mongo/scripting/engine.h @@ -191,20 +191,6 @@ public: return _lastRetIsNativeCode; } - class NoDBAccess { - Scope* _s; - - public: - NoDBAccess(Scope* s) : _s(s) {} - ~NoDBAccess() { - _s->rename("____db____", "db"); - } - }; - NoDBAccess disableDBAccess(const char* why) { - rename("db", "____db____"); - return NoDBAccess(this); - } - protected: friend class PooledScope; diff --git a/src/mongo/scripting/mozjs/implscope.cpp b/src/mongo/scripting/mozjs/implscope.cpp index 9f201cc3dc7..21b29841d52 100644 --- a/src/mongo/scripting/mozjs/implscope.cpp +++ b/src/mongo/scripting/mozjs/implscope.cpp @@ -198,10 +198,15 @@ bool MozJSImplScope::_interruptCallback(JSContext* cx) { // Check our initial kill status (which might be fine). auto status = [&scope]() -> Status { stdx::lock_guard<stdx::mutex> lk(scope->_mutex); + + if (scope->_inOp > 0 && scope->_opCtx) { + scope->_killStatus = scope->_opCtx->checkForInterruptNoAssert(); + } + return scope->_killStatus; }(); - if (scope->_hasOutOfMemoryException) { + if (status.isOK() && scope->_hasOutOfMemoryException) { status = Status(ErrorCodes::JSInterpreterFailure, "Out of memory"); } @@ -781,6 +786,17 @@ void MozJSImplScope::gc() { void MozJSImplScope::sleep(Milliseconds ms) { stdx::unique_lock<stdx::mutex> lk(_mutex); + if (_opCtx) { + try { + _opCtx->waitForConditionOrInterruptFor(_sleepCondition, lk, ms); + } catch (const DBException& ex) { + _killStatus = ex.toStatus(); + uasserted(ErrorCodes::JSUncatchableError, "sleep was interrupted by kill"); + } + + return; + } + uassert(ErrorCodes::JSUncatchableError, "sleep was interrupted by kill", !_sleepCondition.wait_for( diff --git a/src/mongo/scripting/mozjs/implscope.h b/src/mongo/scripting/mozjs/implscope.h index 83f8981a683..db9ba853eba 100644 --- a/src/mongo/scripting/mozjs/implscope.h +++ b/src/mongo/scripting/mozjs/implscope.h @@ -415,7 +415,7 @@ private: InternedStringTable _internedStrings; Status _killStatus; mutable std::mutex _mutex; - std::condition_variable _sleepCondition; + stdx::condition_variable _sleepCondition; std::string _error; unsigned int _opId; // op id for this scope OperationContext* _opCtx; // Op context for DbEval diff --git a/src/mongo/scripting/mozjs/proxyscope.cpp b/src/mongo/scripting/mozjs/proxyscope.cpp index 8add7340d6a..584958eb6bf 100644 --- a/src/mongo/scripting/mozjs/proxyscope.cpp +++ b/src/mongo/scripting/mozjs/proxyscope.cpp @@ -39,6 +39,7 @@ #include "mongo/scripting/mozjs/implscope.h" #include "mongo/util/concurrency/idle_thread_block.h" #include "mongo/util/destructor_guard.h" +#include "mongo/util/functional.h" #include "mongo/util/quick_exit.h" #include "mongo/util/scopeguard.h" @@ -66,7 +67,7 @@ MozJSProxyScope::MozJSProxyScope(MozJSScriptEngine* engine) // Test the child on startup to make sure it's awake and that the // implementation scope sucessfully constructed. try { - runOnImplThread([] {}); + run([] {}); } catch (...) { shutdownThread(); throw; @@ -82,7 +83,8 @@ void MozJSProxyScope::init(const BSONObj* data) { } void MozJSProxyScope::reset() { - run([&] { _implScope->reset(); }); + unregisterOperation(); + runWithoutInterruption([&] { _implScope->reset(); }); } bool MozJSProxyScope::isKillPending() const { @@ -90,11 +92,11 @@ bool MozJSProxyScope::isKillPending() const { } void MozJSProxyScope::registerOperation(OperationContext* opCtx) { - run([&] { _implScope->registerOperation(opCtx); }); + _opCtx = opCtx; } void MozJSProxyScope::unregisterOperation() { - run([&] { _implScope->unregisterOperation(); }); + _opCtx = nullptr; } void MozJSProxyScope::externalSetup() { @@ -103,13 +105,13 @@ void MozJSProxyScope::externalSetup() { std::string MozJSProxyScope::getError() { std::string out; - run([&] { out = _implScope->getError(); }); + runWithoutInterruption([&] { out = _implScope->getError(); }); return out; } bool MozJSProxyScope::hasOutOfMemoryException() { bool out; - run([&] { out = _implScope->hasOutOfMemoryException(); }); + runWithoutInterruption([&] { out = _implScope->hasOutOfMemoryException(); }); return out; } @@ -118,11 +120,11 @@ void MozJSProxyScope::gc() { } void MozJSProxyScope::advanceGeneration() { - run([&] { _implScope->advanceGeneration(); }); + runWithoutInterruption([&] { _implScope->advanceGeneration(); }); } void MozJSProxyScope::requireOwnedObjects() { - run([&] { _implScope->requireOwnedObjects(); }); + runWithoutInterruption([&] { _implScope->requireOwnedObjects(); }); } double MozJSProxyScope::getNumber(const char* field) { @@ -240,10 +242,6 @@ ScriptingFunction MozJSProxyScope::_createFunction(const char* raw) { return out; } -OperationContext* MozJSProxyScope::getOpContext() const { - return _implScope->getOpContext(); -} - void MozJSProxyScope::kill() { _implScope->kill(); } @@ -255,7 +253,7 @@ void MozJSProxyScope::interrupt() { /** * Invokes a function on the implementation thread * - * It does this by serializing the invocation through a stdx::function and + * It does this by serializing the invocation through a unique_function and * capturing any exceptions through _status. * * We transition: @@ -276,7 +274,18 @@ void MozJSProxyScope::run(Closure&& closure) { runOnImplThread(std::move(closure)); } -void MozJSProxyScope::runOnImplThread(stdx::function<void()> f) { +template <typename Closure> +void MozJSProxyScope::runWithoutInterruption(Closure&& closure) { + auto toRun = [&] { run(std::forward<Closure>(closure)); }; + + if (_opCtx) { + return _opCtx->runWithoutInterruption(toRun); + } else { + return toRun(); + } +} + +void MozJSProxyScope::runOnImplThread(unique_function<void()> f) { stdx::unique_lock<stdx::mutex> lk(_mutex); _function = std::move(f); @@ -285,7 +294,18 @@ void MozJSProxyScope::runOnImplThread(stdx::function<void()> f) { _condvar.notify_one(); - _condvar.wait(lk, [this] { return _state == State::ImplResponse; }); + Interruptible* interruptible = _opCtx ? _opCtx : Interruptible::notInterruptible(); + + auto pred = [&] { return _state == State::ImplResponse; }; + + try { + interruptible->waitForConditionOrInterrupt(_condvar, lk, pred); + } catch (const DBException& ex) { + _status = ex.toStatus(); + + _implScope->kill(); + _condvar.wait(lk, pred); + } _state = State::Idle; @@ -359,6 +379,8 @@ void MozJSProxyScope::implThread(void* arg) { break; try { + lk.unlock(); + const auto unlockGuard = makeGuard([&] { lk.lock(); }); proxy->_function(); } catch (...) { proxy->_status = exceptionToStatus(); diff --git a/src/mongo/scripting/mozjs/proxyscope.h b/src/mongo/scripting/mozjs/proxyscope.h index 42d034673a6..3afd17c6dda 100644 --- a/src/mongo/scripting/mozjs/proxyscope.h +++ b/src/mongo/scripting/mozjs/proxyscope.h @@ -35,9 +35,9 @@ #include "mongo/client/dbclient_cursor.h" #include "mongo/scripting/mozjs/engine.h" #include "mongo/stdx/condition_variable.h" -#include "mongo/stdx/functional.h" #include "mongo/stdx/mutex.h" #include "mongo/stdx/thread.h" +#include "mongo/util/functional.h" namespace mongo { namespace mozjs { @@ -51,7 +51,7 @@ class MozJSImplScope; * implementation scope from multiple threads. * * In terms of implementation, it does most of it's heavy lifting through a - * stdx::function. The proxy scope owns an implementation scope transitively + * unique_function. The proxy scope owns an implementation scope transitively * through the thread it owns. They communicate by setting a variable, then * signaling each other. That communication has to work, there's no fallback * for timing out. @@ -174,15 +174,16 @@ public: ScriptingFunction _createFunction(const char* code) override; - OperationContext* getOpContext() const; - void interrupt(); private: template <typename Closure> void run(Closure&& closure); - void runOnImplThread(stdx::function<void()> f); + template <typename Closure> + void runWithoutInterruption(Closure&& closure); + + void runOnImplThread(unique_function<void()> f); void shutdownThread(); static void implThread(void* proxy); @@ -195,9 +196,10 @@ private: * function invocation and exception handling */ stdx::mutex _mutex; - stdx::function<void()> _function; + unique_function<void()> _function; State _state; Status _status; + OperationContext* _opCtx = nullptr; stdx::condition_variable _condvar; PRThread* _thread; |