From a177a4691b146704279ab2e73168a01c18be02c2 Mon Sep 17 00:00:00 2001 From: Ted Tuckman Date: Thu, 15 Jun 2017 16:29:10 -0400 Subject: SERVER-29651 Interrupts are caught in implscope functions or by the deadline monitor if neccessary. (cherry picked from commit ce5e5fdfcad9ee0b3b08954982ded2a4bdfd8ac2) --- src/mongo/scripting/deadline_monitor.cpp | 2 +- src/mongo/scripting/deadline_monitor.h | 6 +- src/mongo/scripting/deadline_monitor_test.cpp | 13 ++ src/mongo/scripting/engine.cpp | 13 ++ src/mongo/scripting/mozjs/implscope.cpp | 176 +++++++++++++------------- src/mongo/scripting/mozjs/implscope.h | 3 + 6 files changed, 120 insertions(+), 93 deletions(-) (limited to 'src') diff --git a/src/mongo/scripting/deadline_monitor.cpp b/src/mongo/scripting/deadline_monitor.cpp index f75c34a0cc2..5eb0f52e5de 100644 --- a/src/mongo/scripting/deadline_monitor.cpp +++ b/src/mongo/scripting/deadline_monitor.cpp @@ -34,7 +34,7 @@ namespace mongo { -MONGO_EXPORT_SERVER_PARAMETER(scriptingEngineInterruptIntervalMS, int, 0); +MONGO_EXPORT_SERVER_PARAMETER(scriptingEngineInterruptIntervalMS, int, 1000); int getScriptingEngineInterruptInterval() { return scriptingEngineInterruptIntervalMS.load(); diff --git a/src/mongo/scripting/deadline_monitor.h b/src/mongo/scripting/deadline_monitor.h index b3c5007c72d..9081d7366e6 100644 --- a/src/mongo/scripting/deadline_monitor.h +++ b/src/mongo/scripting/deadline_monitor.h @@ -141,10 +141,10 @@ private: const Date_t now = Date_t::now(); const auto interruptInterval = Milliseconds{getScriptingEngineInterruptInterval()}; - if ((interruptInterval.count() > 0) && (now - lastInterruptCycle > interruptInterval)) { + if (now - lastInterruptCycle > interruptInterval) { for (const auto& task : _tasks) { - if (task.second > now) - task.first->interrupt(); + if (task.first->isKillPending()) + task.first->kill(); } lastInterruptCycle = now; } diff --git a/src/mongo/scripting/deadline_monitor_test.cpp b/src/mongo/scripting/deadline_monitor_test.cpp index 71daefbce8f..d802673bd3d 100644 --- a/src/mongo/scripting/deadline_monitor_test.cpp +++ b/src/mongo/scripting/deadline_monitor_test.cpp @@ -73,8 +73,12 @@ public: _group->noteKill(); } void interrupt() {} + const bool isKillPending() { + return killPending; + } TaskGroup* _group; uint64_t _killed; + bool killPending = false; }; // single task expires before stopping the deadline @@ -175,4 +179,13 @@ TEST(DeadlineMonitor, MultipleTasksExpireOrComplete) { } } +TEST(DeadlineMonitor, IsKillPendingKills) { + DeadlineMonitor dm; + TaskGroup group; + Task task(&group); + dm.startDeadline(&task, -1); + task.killPending = true; + group.waitForKillCount(1); + ASSERT(task._killed); +} } // namespace mongo diff --git a/src/mongo/scripting/engine.cpp b/src/mongo/scripting/engine.cpp index 684d4d747c9..1848bccfba2 100644 --- a/src/mongo/scripting/engine.cpp +++ b/src/mongo/scripting/engine.cpp @@ -42,6 +42,7 @@ #include "mongo/db/service_context.h" #include "mongo/platform/unordered_set.h" #include "mongo/scripting/dbdirectclient_factory.h" +#include "mongo/util/fail_point_service.h" #include "mongo/util/file.h" #include "mongo/util/log.h" #include "mongo/util/text.h" @@ -55,7 +56,10 @@ using std::unique_ptr; AtomicInt64 Scope::_lastVersion(1); + namespace { + +MONGO_FP_DECLARE(mr_killop_test_fp); // 2 GB is the largest support Javascript file size. const fileofs kMaxJsFileLength = fileofs(2) * 1024 * 1024 * 1024; @@ -231,6 +235,15 @@ void Scope::loadStored(OperationContext* txn, bool ignoreNotConnected) { uassert(10209, str::stream() << "name has to be a string: " << n, n.type() == String); uassert(10210, "value has to be set", v.type() != EOO); + if (MONGO_FAIL_POINT(mr_killop_test_fp)) { + + /* This thread sleep makes the interrupts in the test come in at a time + * where the js misses the interrupt and throw an exception instead of + * being interrupted + */ + stdx::this_thread::sleep_for(stdx::chrono::seconds(1)); + } + try { setElement(n.valuestr(), v, o); thisTime.insert(n.valuestr()); diff --git a/src/mongo/scripting/mozjs/implscope.cpp b/src/mongo/scripting/mozjs/implscope.cpp index eb8ad255167..c31d5b940a4 100644 --- a/src/mongo/scripting/mozjs/implscope.cpp +++ b/src/mongo/scripting/mozjs/implscope.cpp @@ -499,82 +499,81 @@ void MozJSImplScope::init(const BSONObj* data) { } } -void MozJSImplScope::setNumber(const char* field, double val) { - MozJSEntry entry(this); +template +auto MozJSImplScope::_runSafely(ImplScopeFunction&& functionToRun) -> decltype(functionToRun()) { + try { + MozJSEntry entry(this); + return functionToRun(); + } catch (...) { + _error = _status.reason(); + + // Clear the status state + auto status = std::move(_status); + uassertStatusOK(status); + throw; + } +} - ObjectWrapper(_context, _global).setNumber(field, val); +void MozJSImplScope::setNumber(const char* field, double val) { + _runSafely([this, &field, &val] { ObjectWrapper(_context, _global).setNumber(field, val); }); } void MozJSImplScope::setString(const char* field, StringData val) { - MozJSEntry entry(this); - - ObjectWrapper(_context, _global).setString(field, val); + _runSafely([this, &field, &val] { ObjectWrapper(_context, _global).setString(field, val); }); } void MozJSImplScope::setBoolean(const char* field, bool val) { - MozJSEntry entry(this); - - ObjectWrapper(_context, _global).setBoolean(field, val); + _runSafely([this, &field, &val] { ObjectWrapper(_context, _global).setBoolean(field, val); }); } void MozJSImplScope::setElement(const char* field, const BSONElement& e, const BSONObj& parent) { - MozJSEntry entry(this); + _runSafely([this, &field, &e, &parent] { - ObjectWrapper(_context, _global).setBSONElement(field, e, parent, false); + ObjectWrapper(_context, _global).setBSONElement(field, e, parent, false); + }); } void MozJSImplScope::setObject(const char* field, const BSONObj& obj, bool readOnly) { - MozJSEntry entry(this); + _runSafely([this, &field, &obj, &readOnly] { - ObjectWrapper(_context, _global).setBSON(field, obj, readOnly); + ObjectWrapper(_context, _global).setBSON(field, obj, readOnly); + }); } int MozJSImplScope::type(const char* field) { - MozJSEntry entry(this); - - return ObjectWrapper(_context, _global).type(field); + return _runSafely([this, &field] { return ObjectWrapper(_context, _global).type(field); }); } double MozJSImplScope::getNumber(const char* field) { - MozJSEntry entry(this); - - return ObjectWrapper(_context, _global).getNumber(field); + return _runSafely([this, &field] { return ObjectWrapper(_context, _global).getNumber(field); }); } int MozJSImplScope::getNumberInt(const char* field) { - MozJSEntry entry(this); - - return ObjectWrapper(_context, _global).getNumberInt(field); + return _runSafely( + [this, &field] { return ObjectWrapper(_context, _global).getNumberInt(field); }); } long long MozJSImplScope::getNumberLongLong(const char* field) { - MozJSEntry entry(this); - - return ObjectWrapper(_context, _global).getNumberLongLong(field); + return _runSafely( + [this, &field] { return ObjectWrapper(_context, _global).getNumberLongLong(field); }); } Decimal128 MozJSImplScope::getNumberDecimal(const char* field) { - MozJSEntry entry(this); - - return ObjectWrapper(_context, _global).getNumberDecimal(field); + return _runSafely( + [this, &field] { return ObjectWrapper(_context, _global).getNumberDecimal(field); }); } std::string MozJSImplScope::getString(const char* field) { - MozJSEntry entry(this); - - return ObjectWrapper(_context, _global).getString(field); + return _runSafely([this, &field] { return ObjectWrapper(_context, _global).getString(field); }); } bool MozJSImplScope::getBoolean(const char* field) { - MozJSEntry entry(this); - - return ObjectWrapper(_context, _global).getBoolean(field); + return _runSafely( + [this, &field] { return ObjectWrapper(_context, _global).getBoolean(field); }); } BSONObj MozJSImplScope::getObject(const char* field) { - MozJSEntry entry(this); - - return ObjectWrapper(_context, _global).getObject(field); + return _runSafely([this, &field] { return ObjectWrapper(_context, _global).getObject(field); }); } void MozJSImplScope::newFunction(StringData raw, JS::MutableHandleValue out) { @@ -650,17 +649,15 @@ ScriptingFunction MozJSImplScope::_createFunction(const char* raw) { } void MozJSImplScope::setFunction(const char* field, const char* code) { - MozJSEntry entry(this); - - JS::RootedValue fun(_context); - _MozJSCreateFunction(code, &fun); - ObjectWrapper(_context, _global).setValue(field, fun); + _runSafely([this, &field, &code] { + JS::RootedValue fun(_context); + _MozJSCreateFunction(code, &fun); + ObjectWrapper(_context, _global).setValue(field, fun); + }); } void MozJSImplScope::rename(const char* from, const char* to) { - MozJSEntry entry(this); - - ObjectWrapper(_context, _global).rename(from, to); + _runSafely([this, &from, &to] { ObjectWrapper(_context, _global).rename(from, to); }); } int MozJSImplScope::invoke(ScriptingFunction func, @@ -772,15 +769,15 @@ bool MozJSImplScope::exec(StringData code, } void MozJSImplScope::injectNative(const char* field, NativeFunction func, void* data) { - MozJSEntry entry(this); - - JS::RootedObject obj(_context); + _runSafely([this, &field, &func, &data] { + JS::RootedObject obj(_context); - NativeFunctionInfo::make(_context, &obj, func, data); + NativeFunctionInfo::make(_context, &obj, func, data); - JS::RootedValue value(_context); - value.setObjectOrNull(obj); - ObjectWrapper(_context, _global).setValue(field, value); + JS::RootedValue value(_context); + value.setObjectOrNull(obj); + ObjectWrapper(_context, _global).setValue(field, value); + }); } void MozJSImplScope::gc() { @@ -789,57 +786,58 @@ void MozJSImplScope::gc() { } void MozJSImplScope::localConnectForDbEval(OperationContext* txn, const char* dbName) { - MozJSEntry entry(this); - - if (_connectState == ConnectState::External) - uasserted(12510, "externalSetup already called, can't call localConnect"); - if (_connectState == ConnectState::Local) { - if (_localDBName == dbName) - return; - uasserted(12511, - str::stream() << "localConnect previously called with name " << _localDBName); - } + _runSafely([this, &txn, &dbName] { + if (_connectState == ConnectState::External) + uasserted(12510, "externalSetup already called, can't call localConnect"); + if (_connectState == ConnectState::Local) { + if (_localDBName == dbName) + return; + uasserted(12511, + str::stream() << "localConnect previously called with name " << _localDBName); + } - // NOTE: order is important here. the following methods must be called after - // the above conditional statements. + // NOTE: order is important here. the following methods must be called after + // the above conditional statements. - _connectState = ConnectState::Local; - _localDBName = dbName; + _connectState = ConnectState::Local; + _localDBName = dbName; - loadStored(txn); + loadStored(txn); - // install db access functions in the global object - installDBAccess(); + // install db access functions in the global object + installDBAccess(); - // install the Mongo function object and instantiate the 'db' global - _mongoLocalProto.install(_global); - execCoreFiles(); + // install the Mongo function object and instantiate the 'db' global + _mongoLocalProto.install(_global); + execCoreFiles(); - const char* const makeMongo = "const _mongo = new Mongo()"; - exec(makeMongo, "local connect 2", false, true, true, 0); + const char* const makeMongo = "const _mongo = new Mongo()"; + exec(makeMongo, "local connect 2", false, true, true, 0); - std::string makeDB = str::stream() << "const db = _mongo.getDB(\"" << dbName << "\");"; - exec(makeDB, "local connect 3", false, true, true, 0); + std::string makeDB = str::stream() << "const db = _mongo.getDB(\"" << dbName << "\");"; + exec(makeDB, "local connect 3", false, true, true, 0); + }); } void MozJSImplScope::externalSetup() { - MozJSEntry entry(this); - if (_connectState == ConnectState::External) - return; - if (_connectState == ConnectState::Local) - uasserted(12512, "localConnect already called, can't call externalSetup"); + _runSafely([&] { + if (_connectState == ConnectState::External) + return; + if (_connectState == ConnectState::Local) + uasserted(12512, "localConnect already called, can't call externalSetup"); - // install db access functions in the global object - installDBAccess(); + // install db access functions in the global object + installDBAccess(); - // install thread-related functions (e.g. _threadInject) - installFork(); + // install thread-related functions (e.g. _threadInject) + installFork(); - // install the Mongo function object - _mongoExternalProto.install(_global); - execCoreFiles(); - _connectState = ConnectState::External; + // install the Mongo function object + _mongoExternalProto.install(_global); + execCoreFiles(); + _connectState = ConnectState::External; + }); } void MozJSImplScope::reset() { diff --git a/src/mongo/scripting/mozjs/implscope.h b/src/mongo/scripting/mozjs/implscope.h index ea9b35d2cf4..2de309e87ab 100644 --- a/src/mongo/scripting/mozjs/implscope.h +++ b/src/mongo/scripting/mozjs/implscope.h @@ -345,6 +345,9 @@ public: }; private: + template + auto _runSafely(ImplScopeFunction&& functionToRun) -> decltype(functionToRun()); + void _MozJSCreateFunction(StringData raw, JS::MutableHandleValue fun); /** -- cgit v1.2.1