SERVER-35061: Javascript sleep should error on premature wake

7 files changed, 92 insertions, 29 deletions

diff --git a/src/mongo/dbtests/jstests.cpp b/src/mongo/dbtests/jstests.cpp
index e928f54b1da..ba43b2802e9 100644
--- a/src/mongo/dbtests/jstests.cpp
+++ b/src/mongo/dbtests/jstests.cpp
@@ -46,7 +46,9 @@
 #include "mongo/scripting/engine.h"
 #include "mongo/shell/shell_utils.h"
 #include "mongo/util/concurrency/thread_name.h"
+#include "mongo/util/future.h"
 #include "mongo/util/log.h"
+#include "mongo/util/time_support.h"
 #include "mongo/util/timer.h"
 
 using std::cout;
@@ -952,6 +954,57 @@ public:
     }
 };
 
+class SleepInterruption {
+public:
+    void run() {
+        std::shared_ptr<Scope> scope(getGlobalScriptEngine()->newScope());
+
+        Promise<void> sleepPromise{};
+        auto sleepFuture = sleepPromise.getFuture();
+
+        Promise<void> awakenedPromise{};
+        auto awakenedFuture = awakenedPromise.getFuture();
+
+        // Spawn a thread which attempts to sleep indefinitely.
+        stdx::thread([
+            preSleep = std::move(sleepPromise),
+            onAwake = std::move(awakenedPromise),
+            scope
+        ]() mutable {
+            preSleep.emplaceValue();
+            onAwake.setWith([&] {
+                scope->exec(
+                    ""
+                    "  try {"
+                    "    sleep(99999999999);"
+                    "  } finally {"
+                    "    throw \"FAILURE\";"
+                    "  }"
+                    "",
+                    "test",
+                    false,
+                    false,
+                    true);
+            });
+        }).detach();
+
+        // Wait until just before the sleep begins.
+        sleepFuture.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
+        // death will emit an error with the Interrupted code.
+        sleepsecs(1);
+
+        // Send the operation a kill signal.
+        scope->kill();
+
+        // Wait for the error.
+        auto result = awakenedFuture.getNoThrow();
+        ASSERT_EQ(ErrorCodes::Interrupted, result);
+    }
+};
+
 /**
  * Test invoke() timeout value does not terminate execution (SERVER-8053)
  */
@@ -2547,6 +2600,7 @@ public:
         add<ExecTimeout>();
         add<ExecNoTimeout>();
         add<InvokeTimeout>();
+        add<SleepInterruption>();
         add<InvokeNoTimeout>();
 
         add<ObjectMapping>();
diff --git a/src/mongo/scripting/engine.cpp b/src/mongo/scripting/engine.cpp
index e22192fe983..31784ad50a0 100644
--- a/src/mongo/scripting/engine.cpp
+++ b/src/mongo/scripting/engine.cpp
@@ -440,6 +440,9 @@ public:
     void requireOwnedObjects() override {
         _real->requireOwnedObjects();
     }
+    void kill() {
+        _real->kill();
+    }
     bool isKillPending() const {
         return _real->isKillPending();
     }
diff --git a/src/mongo/scripting/engine.h b/src/mongo/scripting/engine.h
index 0792c95eb7d..2ed64157e01 100644
--- a/src/mongo/scripting/engine.h
+++ b/src/mongo/scripting/engine.h
@@ -96,6 +96,8 @@ public:
 
     virtual bool hasOutOfMemoryException() = 0;
 
+    virtual void kill() = 0;
+
     virtual bool isKillPending() const = 0;
 
     virtual void gc() = 0;
diff --git a/src/mongo/scripting/mozjs/exception.cpp b/src/mongo/scripting/mozjs/exception.cpp
index d32ea8eb55a..100ea4c47dd 100644
--- a/src/mongo/scripting/mozjs/exception.cpp
+++ b/src/mongo/scripting/mozjs/exception.cpp
@@ -43,31 +43,20 @@
 namespace mongo {
 namespace mozjs {
 
-namespace {
-
-JSErrorFormatString kErrorFormatString = {"{0}", 1, JSEXN_ERR};
-const JSErrorFormatString* errorCallback(void* data, const unsigned code) {
-    return &kErrorFormatString;
-}
-
-JSErrorFormatString kUncatchableErrorFormatString = {"{0}", 1, JSEXN_NONE};
-const JSErrorFormatString* uncatchableErrorCallback(void* data, const unsigned code) {
-    return &kUncatchableErrorFormatString;
-}
-
-MONGO_STATIC_ASSERT_MSG(
-    UINT_MAX - JSErr_Limit > ErrorCodes::MaxError,
-    "Not enough space in an unsigned int for Mongo ErrorCodes and JSErrorNumbers");
-
-}  // namespace
-
 void mongoToJSException(JSContext* cx) {
     auto status = exceptionToStatus();
 
-    JS::RootedValue val(cx);
-    statusToJSException(cx, status, &val);
+    if (status.code() != ErrorCodes::JSUncatchableError) {
+        JS::RootedValue val(cx);
+        statusToJSException(cx, status, &val);
 
-    JS_SetPendingException(cx, val);
+        JS_SetPendingException(cx, val);
+    } else {
+        // If a JSAPI callback returns false without setting a pending exception, SpiderMonkey will
+        // treat it as an uncatchable error.
+        auto scope = getScope(cx);
+        scope->setStatus(status);
+    }
 }
 
 std::string currentJSStackToString(JSContext* cx) {
diff --git a/src/mongo/scripting/mozjs/implscope.cpp b/src/mongo/scripting/mozjs/implscope.cpp
index 249dc489fb1..5c6c150fc00 100644
--- a/src/mongo/scripting/mozjs/implscope.cpp
+++ b/src/mongo/scripting/mozjs/implscope.cpp
@@ -801,7 +801,10 @@ void MozJSImplScope::gc() {
 void MozJSImplScope::sleep(Milliseconds ms) {
     std::unique_lock<std::mutex> lk(_sleepMutex);
 
-    _sleepCondition.wait_for(lk, ms.toSystemDuration(), [this] { return _pendingKill.load(); });
+    uassert(ErrorCodes::JSUncatchableError,
+            "sleep was interrupted by kill",
+            !_sleepCondition.wait_for(
+                lk, ms.toSystemDuration(), [this] { return _pendingKill.load(); }));
 }
 
 void MozJSImplScope::localConnectForDbEval(OperationContext* opCtx, const char* dbName) {
@@ -907,9 +910,19 @@ void MozJSImplScope::installFork() {
     _jsThreadProto.install(_global);
 }
 
+void MozJSImplScope::setStatus(Status status) {
+    _status = std::move(status);
+}
+
 bool MozJSImplScope::_checkErrorState(bool success, bool reportError, bool assertOnError) {
-    if (success)
+    if (isKillPending()) {
+        success = false;
+        _status = Status(ErrorCodes::Interrupted, "JavaScript execution interrupted");
+    }
+
+    if (success) {
         return false;
+    }
 
     if (_status.isOK()) {
         JS::RootedValue excn(_context);
diff --git a/src/mongo/scripting/mozjs/implscope.h b/src/mongo/scripting/mozjs/implscope.h
index 6017ca55b3c..a8af88e8549 100644
--- a/src/mongo/scripting/mozjs/implscope.h
+++ b/src/mongo/scripting/mozjs/implscope.h
@@ -91,7 +91,7 @@ public:
 
     void reset() override;
 
-    void kill();
+    void kill() override;
 
     void interrupt();
 
@@ -364,6 +364,8 @@ public:
         static ASANHandles* getThreadASANHandles();
     };
 
+    void setStatus(Status status);
+
 private:
     template <typename ImplScopeFunction>
     auto _runSafely(ImplScopeFunction&& functionToRun) -> decltype(functionToRun());
diff --git a/src/mongo/scripting/mozjs/proxyscope.h b/src/mongo/scripting/mozjs/proxyscope.h
index 9e779fa9ee9..1e3fe69999b 100644
--- a/src/mongo/scripting/mozjs/proxyscope.h
+++ b/src/mongo/scripting/mozjs/proxyscope.h
@@ -111,6 +111,11 @@ public:
 
     void reset() override;
 
+    /**
+     * Thread safe. Kills the running operation
+     */
+    void kill() override;
+
     bool isKillPending() const override;
 
     void registerOperation(OperationContext* opCtx) override;
@@ -171,11 +176,6 @@ public:
 
     OperationContext* getOpContext() const;
 
-    /**
-     * Thread safe. Kills the running operation
-     */
-    void kill();
-
     void interrupt();
 
 private: