SERVER-29651 Interrupts are caught in implscope functions or by the deadline monitor if neccessary.

(cherry picked from commit ce5e5fdfcad9ee0b3b08954982ded2a4bdfd8ac2)

6 files changed, 121 insertions, 95 deletions

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 05acd5c349c..b509d4fb480 100644
--- a/src/mongo/scripting/deadline_monitor.h
+++ b/src/mongo/scripting/deadline_monitor.h
@@ -138,10 +138,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<Task> 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 71c42f2fe74..a1515f23830 100644
--- a/src/mongo/scripting/engine.cpp
+++ b/src/mongo/scripting/engine.cpp
@@ -41,6 +41,8 @@
 #include "mongo/db/service_context.h"
 #include "mongo/db/operation_context.h"
 #include "mongo/platform/unordered_set.h"
+#include "mongo/stdx/thread.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 +57,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;
 }  // namespace
@@ -226,6 +231,14 @@ 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 e85efd97505..f6789d0c5cf 100644
--- a/src/mongo/scripting/mozjs/implscope.cpp
+++ b/src/mongo/scripting/mozjs/implscope.cpp
@@ -407,82 +407,81 @@ void MozJSImplScope::init(const BSONObj* data) {
     }
 }
 
-void MozJSImplScope::setNumber(const char* field, double val) {
-    MozJSEntry entry(this);
+template <typename ImplScopeFunction>
+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) {
@@ -552,17 +551,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,
@@ -674,15 +671,15 @@ bool MozJSImplScope::exec(StringData code,
 }
 
 void MozJSImplScope::injectNative(const char* field, NativeFunction func, void* data) {
-    MozJSEntry entry(this);
+    _runSafely([this, &field, &func, &data] {
+        JS::RootedObject obj(_context);
 
-    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() {
@@ -691,59 +688,59 @@ 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");
 
-    mongo::sm::reset(0);
+        mongo::sm::reset(0);
 
-    // 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 edfc236e90c..f64f6cfbdf8 100644
--- a/src/mongo/scripting/mozjs/implscope.h
+++ b/src/mongo/scripting/mozjs/implscope.h
@@ -314,6 +314,9 @@ public:
     }
 
 private:
+    template <typename ImplScopeFunction>
+    auto _runSafely(ImplScopeFunction&& functionToRun) -> decltype(functionToRun());
+
     void _MozJSCreateFunction(StringData raw, JS::MutableHandleValue fun);
 
     /**