From 21b5477520744ca02e1574160caa31e0633849dd Mon Sep 17 00:00:00 2001 From: Jason Carey Date: Thu, 3 Jan 2019 16:38:26 -0500 Subject: SERVER-39183 honor socket disconnect in $where --- src/mongo/db/commands/mr.cpp | 1 - src/mongo/db/matcher/expression_where.cpp | 6 + src/mongo/dbtests/jstests.cpp | 244 ++++++++++++++++++------------ src/mongo/scripting/engine.h | 14 -- src/mongo/scripting/mozjs/implscope.cpp | 18 ++- src/mongo/scripting/mozjs/implscope.h | 2 +- src/mongo/scripting/mozjs/proxyscope.cpp | 52 +++++-- src/mongo/scripting/mozjs/proxyscope.h | 14 +- 8 files changed, 214 insertions(+), 137 deletions(-) (limited to 'src') 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 class BuiltinTests { public: void run() { @@ -70,11 +73,12 @@ public: } }; +template class BasicScope { public: void run() { unique_ptr s; - s.reset(getGlobalScriptEngine()->newScope()); + s.reset((getGlobalScriptEngine()->*scopeFactory)()); s->setNumber("x", 5); ASSERT(5 == s->getNumber("x")); @@ -93,12 +97,13 @@ public: } }; +template class ResetScope { public: void run() { /* Currently reset does not clear data in v8 or spidermonkey scopes. See SECURITY-10 unique_ptr s; - s.reset( getGlobalScriptEngine()->newScope() ); + s.reset( (getGlobalScriptEngine()->*scopeFactory)() ); s->setBoolean( "x" , true ); ASSERT( s->getBoolean( "x" ) ); @@ -109,12 +114,13 @@ public: } }; +template class FalseTests { public: void run() { // Test falsy javascript values unique_ptr s; - s.reset(getGlobalScriptEngine()->newScope()); + s.reset((getGlobalScriptEngine()->*scopeFactory)()); ASSERT(!s->getBoolean("notSet")); @@ -133,10 +139,11 @@ public: } }; +template class SimpleFunctions { public: void run() { - unique_ptr s(getGlobalScriptEngine()->newScope()); + unique_ptr 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 class ExecLogError { public: void run() { - unique_ptr scope(getGlobalScriptEngine()->newScope()); + unique_ptr 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 class InvokeLogError { public: void run() { - unique_ptr scope(getGlobalScriptEngine()->newScope()); + unique_ptr 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 class ObjectMapping { public: void run() { - unique_ptr s(getGlobalScriptEngine()->newScope()); + unique_ptr s((getGlobalScriptEngine()->*scopeFactory)()); BSONObj o = BSON("x" << 17.0 << "y" << "eliot" @@ -316,10 +326,11 @@ public: } }; +template class ObjectDecoding { public: void run() { - unique_ptr s(getGlobalScriptEngine()->newScope()); + unique_ptr s((getGlobalScriptEngine()->*scopeFactory)()); s->invoke("z = { num : 1 };", 0, 0); BSONObj out = s->getObject("z"); @@ -338,11 +349,12 @@ public: } }; +template class JSOIDTests { public: void run() { #ifdef MOZJS - unique_ptr s(getGlobalScriptEngine()->newScope()); + unique_ptr s((getGlobalScriptEngine()->*scopeFactory)()); s->localConnect("blah"); @@ -370,10 +382,11 @@ public: } }; +template class SetImplicit { public: void run() { - unique_ptr s(getGlobalScriptEngine()->newScope()); + unique_ptr s((getGlobalScriptEngine()->*scopeFactory)()); BSONObj o = BSON("foo" << "bar"); @@ -392,10 +405,11 @@ public: } }; +template class ObjectModReadonlyTests { public: void run() { - unique_ptr s(getGlobalScriptEngine()->newScope()); + unique_ptr s((getGlobalScriptEngine()->*scopeFactory)()); BSONObj o = BSON("x" << 17 << "y" << "eliot" @@ -430,10 +444,11 @@ public: } }; +template class OtherJSTypes { public: void run() { - unique_ptr s(getGlobalScriptEngine()->newScope()); + unique_ptr s((getGlobalScriptEngine()->*scopeFactory)()); { // date @@ -527,10 +542,11 @@ public: } }; +template class SpecialDBTypes { public: void run() { - unique_ptr s(getGlobalScriptEngine()->newScope()); + unique_ptr s((getGlobalScriptEngine()->*scopeFactory)()); BSONObjBuilder b; b.appendTimestamp("a", 123456789); @@ -561,10 +577,11 @@ public: } }; +template class TypeConservation { public: void run() { - unique_ptr s(getGlobalScriptEngine()->newScope()); + unique_ptr s((getGlobalScriptEngine()->*scopeFactory)()); // -- A -- @@ -659,10 +676,11 @@ public: } }; +template class NumberLong { public: void run() { - unique_ptr s(getGlobalScriptEngine()->newScope()); + unique_ptr s((getGlobalScriptEngine()->*scopeFactory)()); BSONObjBuilder b; long long val = (long long)(0xbabadeadbeefbaddULL); b.append("a", val); @@ -718,10 +736,11 @@ public: } }; +template class NumberLong2 { public: void run() { - unique_ptr s(getGlobalScriptEngine()->newScope()); + unique_ptr s((getGlobalScriptEngine()->*scopeFactory)()); BSONObj in; { @@ -745,10 +764,11 @@ public: } }; +template class NumberLongUnderLimit { public: void run() { - unique_ptr s(getGlobalScriptEngine()->newScope()); + unique_ptr s((getGlobalScriptEngine()->*scopeFactory)()); BSONObjBuilder b; // limit is 2^53 @@ -791,10 +811,11 @@ public: } }; +template class NumberDecimal { public: void run() { - unique_ptr s(getGlobalScriptEngine()->newScope()); + unique_ptr s((getGlobalScriptEngine()->*scopeFactory)()); BSONObjBuilder b; Decimal128 val = Decimal128("2.010"); b.append("a", val); @@ -820,19 +841,21 @@ public: } }; +template class NumberDecimalGetFromScope { public: void run() { - unique_ptr s(getGlobalScriptEngine()->newScope()); + unique_ptr s((getGlobalScriptEngine()->*scopeFactory)()); ASSERT(s->exec("a = 5;", "a", false, true, false)); ASSERT_TRUE(Decimal128(5).isEqual(s->getNumberDecimal("a"))); } }; +template class NumberDecimalBigObject { public: void run() { - unique_ptr s(getGlobalScriptEngine()->newScope()); + unique_ptr s((getGlobalScriptEngine()->*scopeFactory)()); BSONObj in; { @@ -856,10 +879,11 @@ public: } }; +template class MaxTimestamp { public: void run() { - unique_ptr s(getGlobalScriptEngine()->newScope()); + unique_ptr s((getGlobalScriptEngine()->*scopeFactory)()); // Timestamp 't' component can exceed max for int32_t. BSONObj in; @@ -877,6 +901,7 @@ public: } }; +template class WeirdObjects { public: BSONObj build(int depth) { @@ -888,7 +913,7 @@ public: } void run() { - unique_ptr s(getGlobalScriptEngine()->newScope()); + unique_ptr 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 class ExecTimeout { public: void run() { - unique_ptr scope(getGlobalScriptEngine()->newScope()); + unique_ptr 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 class ExecNoTimeout { public: void run() { - unique_ptr scope(getGlobalScriptEngine()->newScope()); + unique_ptr 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 class InvokeTimeout { public: void run() { - unique_ptr scope(getGlobalScriptEngine()->newScope()); + unique_ptr scope((getGlobalScriptEngine()->*scopeFactory)()); // scope timeout after 500ms bool caught = false; @@ -956,22 +984,18 @@ public: } }; +template class SleepInterruption { public: void run() { - std::shared_ptr scope(getGlobalScriptEngine()->newScope()); - - auto sleep = makePromiseFuture(); - auto awakened = makePromiseFuture(); + auto scopePF = makePromiseFuture(); + auto awakenedPF = makePromiseFuture(); // 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((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 class InvokeNoTimeout { public: void run() { - unique_ptr scope(getGlobalScriptEngine()->newScope()); + unique_ptr scope((getGlobalScriptEngine()->*scopeFactory)()); // invoke completes before timeout scope->invokeSafe( @@ -1024,6 +1051,7 @@ public: }; +template class Utf8Check { public: Utf8Check() { @@ -1059,6 +1087,7 @@ private: }; +template class BinDataType { public: void pp(const char* s, BSONElement e) { @@ -1072,7 +1101,7 @@ public: } void run() { - unique_ptr s(getGlobalScriptEngine()->newScope()); + unique_ptr s((getGlobalScriptEngine()->*scopeFactory)()); const char* foo = "asdas\0asdasd"; const char* base64 = "YXNkYXMAYXNkYXNk"; @@ -1119,10 +1148,11 @@ public: } }; +template class VarTests { public: void run() { - unique_ptr s(getGlobalScriptEngine()->newScope()); + unique_ptr s((getGlobalScriptEngine()->*scopeFactory)()); ASSERT(s->exec("a = 5;", "a", false, true, false)); ASSERT_EQUALS(5, s->getNumber("a")); @@ -1132,6 +1162,7 @@ public: } }; +template class Speed1 { public: void run() { @@ -1139,7 +1170,7 @@ public: BSONObj empty; unique_ptr s; - s.reset(getGlobalScriptEngine()->newScope()); + s.reset((getGlobalScriptEngine()->*scopeFactory)()); ScriptingFunction f = s->createFunction("return this.x + 6;"); @@ -1153,11 +1184,12 @@ public: } }; +template class ScopeOut { public: void run() { unique_ptr s; - s.reset(getGlobalScriptEngine()->newScope()); + s.reset((getGlobalScriptEngine()->*scopeFactory)()); s->invokeSafe("x = 5;", 0, 0); { @@ -1179,11 +1211,12 @@ public: } }; +template class RenameTest { public: void run() { unique_ptr 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 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 s(getGlobalScriptEngine()->newScope()); + unique_ptr s((getGlobalScriptEngine()->*scopeFactory)()); s->setObject("val", BSONObj(reinterpret_cast(bits)).getOwned()); @@ -1219,10 +1253,11 @@ public: } }; +template class NoReturnSpecified { public: void run() { - unique_ptr s(getGlobalScriptEngine()->newScope()); + unique_ptr s((getGlobalScriptEngine()->*scopeFactory)()); s->invoke("x=5;", 0, 0); ASSERT_EQUALS(5, s->getNumber("__returnValue")); @@ -1265,6 +1300,7 @@ public: } }; +template class RecursiveInvoke { public: static BSONObj callback(const BSONObj& args, void* data) { @@ -1276,7 +1312,7 @@ public: } void run() { - unique_ptr s(getGlobalScriptEngine()->newScope()); + unique_ptr s((getGlobalScriptEngine()->*scopeFactory)()); s->injectNative("foo", callback, s.get()); s->invoke("var x = 1; foo();", 0, 0); @@ -1284,10 +1320,11 @@ public: } }; +template class ErrorCodeFromInvoke { public: void run() { - unique_ptr s(getGlobalScriptEngine()->newScope()); + unique_ptr s((getGlobalScriptEngine()->*scopeFactory)()); { bool threwException = false; @@ -1321,6 +1358,7 @@ public: } }; +template class ErrorWithSidecarFromInvoke { public: void run() { @@ -1329,7 +1367,7 @@ public: return {}; }; - unique_ptr s(getGlobalScriptEngine()->newScope()); + unique_ptr s((getGlobalScriptEngine()->*scopeFactory)()); s->injectNative("foo", sidecarThrowingFunc); @@ -1340,6 +1378,7 @@ public: } }; +template class RequiresOwnedObjects { public: void run() { @@ -1352,14 +1391,14 @@ public: // Ensure that by default we can bind owned and unowned { - unique_ptr s(getGlobalScriptEngine()->newScope()); + unique_ptr 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 s(getGlobalScriptEngine()->newScope()); + unique_ptr s((getGlobalScriptEngine()->*scopeFactory)()); s->requireOwnedObjects(); s->setObject("owned", owned, true); @@ -1383,6 +1422,7 @@ public: } }; +template class ConvertShardKeyToHashed { public: void check(shared_ptr s, const mongo::BSONObj& o) { @@ -1430,7 +1470,7 @@ public: } void run() { - shared_ptr s(getGlobalScriptEngine()->newScope()); + shared_ptr 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 + void setupTestsWithScopeFactory() { + add>(); + add>(); + add>(); + add>(); + add>(); + add>(); + add>(); + add>(); + add>(); + add>(); + add>(); + add>(); + + add>(); + add>(); + add>(); + add>(); + add>(); + add>(); + add>(); + add>(); + add>(); + add>(); + + add>(); + add>(); + add>(); + + add>(); + add>(); + + add>(); + add>(); + + add>(); + add>(); + add>(); + add>(); + add>(); + add>(); + + add>(); + add>(); + add>(); + add>(); + add>(); + } + void setupTests() { - add(); - add(); - add(); - add(); - add(); - add(); - add(); - add(); - add(); - add(); - add(); - add(); - - add(); - add(); - add(); - add(); - add(); - add(); - add(); - add(); - add(); - add(); - - add(); - add(); - add(); - - add(); - add(); - - add(); - add(); - - add(); - add(); - add(); - add(); - add(); - add(); - - add(); - add(); - add(); - add(); - add(); + 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 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 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 f) { +template +void MozJSProxyScope::runWithoutInterruption(Closure&& closure) { + auto toRun = [&] { run(std::forward(closure)); }; + + if (_opCtx) { + return _opCtx->runWithoutInterruption(toRun); + } else { + return toRun(); + } +} + +void MozJSProxyScope::runOnImplThread(unique_function f) { stdx::unique_lock lk(_mutex); _function = std::move(f); @@ -285,7 +294,18 @@ void MozJSProxyScope::runOnImplThread(stdx::function 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 void run(Closure&& closure); - void runOnImplThread(stdx::function f); + template + void runWithoutInterruption(Closure&& closure); + + void runOnImplThread(unique_function f); void shutdownThread(); static void implThread(void* proxy); @@ -195,9 +196,10 @@ private: * function invocation and exception handling */ stdx::mutex _mutex; - stdx::function _function; + unique_function _function; State _state; Status _status; + OperationContext* _opCtx = nullptr; stdx::condition_variable _condvar; PRThread* _thread; -- cgit v1.2.1