[couchdb] branch hard-crash-on-internal-error-in-couch-js created (now 4a66ebe95)

[va...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

vatamane pushed a change to branch hard-crash-on-internal-error-in-couch-js
in repository https://gitbox.apache.org/repos/asf/couchdb.git


      at 4a66ebe95 Treat javascript internal errors as fatal

This branch includes the following new commits:

     new 4a66ebe95 Treat javascript internal errors as fatal

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.

[couchdb] 01/01: Treat javascript internal errors as fatal

[va...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

vatamane pushed a commit to branch hard-crash-on-internal-error-in-couch-js
in repository https://gitbox.apache.org/repos/asf/couchdb.git

commit 4a66ebe95372fc2a1b86a5f7d423f95f751116f4
Author: Nick Vatamaniuc <va...@gmail.com>
AuthorDate: Wed Mar 29 15:25:18 2023 -0400

    Treat javascript internal errors as fatal
    
    Spidermonkey sometimes throws an `InternalError` when exceeding memory limits,
    when normally we'd expect it to crash or exit with a non-0 exit code. Because
    we trap exceptions, and continue emitting rows, it is possible for users views
    to randomly miss indexed rows based on whether GC had run or not, other
    internal runtime state which may have been consuming more or less memory until
    that time.
    
    To prevent the view continuing processing documents, and randomly dropping
    emitted rows, depending on memory pressure in the JS runtime at the time,
    choose to treat Internal errors as fatal.
    
    After an InternalError is raised we expect the process to exit just like it
    would during OOM.
    
    Add a test to assert this happens.
    
    Fix https://github.com/apache/couchdb/issues/4504
---
 share/server/loop.js                    |  3 +++
 share/server/views.js                   |  2 ++
 src/couch/test/eunit/couch_js_tests.erl | 38 ++++++++++++++++++++++++++++++++-
 3 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/share/server/loop.js b/share/server/loop.js
index 70a143a45..2e6114d99 100644
--- a/share/server/loop.js
+++ b/share/server/loop.js
@@ -133,6 +133,9 @@ var Loop = function() {
     } else if (e.error && e.reason) {
       // compatibility with old error format
       respond(["error", e.error, e.reason]);
+    } else if (e.name == "InternalError") {
+      respond(["error", e.name, e.message]);
+      quit(-1);
     } else if (e.name) {
       respond(["error", e.name, e]);
     } else {
diff --git a/share/server/views.js b/share/server/views.js
index 32d65e457..57cdaf3a9 100644
--- a/share/server/views.js
+++ b/share/server/views.js
@@ -73,6 +73,8 @@ var Views = (function() {
       // Throwing errors of the form ["fatal","error_key","reason"]
       // will kill the OS process. This is not normally what you want.
       throw(err);
+    } else if (err.name == "InternalError") {
+      throw(["fatal", err.name, err.message]);
     }
     var message = "function raised exception " + err.toSource();
     if (doc) message += " with doc._id " + doc._id;
diff --git a/src/couch/test/eunit/couch_js_tests.erl b/src/couch/test/eunit/couch_js_tests.erl
index 789b36321..afd42bb72 100644
--- a/src/couch/test/eunit/couch_js_tests.erl
+++ b/src/couch/test/eunit/couch_js_tests.erl
@@ -28,7 +28,8 @@ couch_js_test_() ->
                 fun should_roundtrip_modified_utf8/0,
                 fun should_replace_broken_utf16/0,
                 fun should_allow_js_string_mutations/0,
-                {timeout, 60000, fun should_exit_on_oom/0}
+                {timeout, 60000, fun should_exit_on_oom/0},
+                {timeout, 60000, fun should_exit_on_internal_error/0}
             ]
         }
     }.
@@ -236,6 +237,41 @@ should_exit_on_oom() ->
     true = couch_query_servers:proc_prompt(Proc, [<<"add_fun">>, Src]),
     trigger_oom(Proc).
 
+%% erlfmt-ignore
+should_exit_on_internal_error() ->
+    % A different way to trigger OOM which previously used to
+    % throw an InternalError on SM. Check that we still exit on that
+    % type of error
+    Src = <<"
+        function(doc) {
+            function mkstr(l) {
+                var r = 'r';
+                while (r.length < l) {r = r + r;}
+                return r;
+            }
+            var s = mkstr(96*1024*1024);
+            var a = [s,s,s,s,s,s,s,s];
+            var j = JSON.stringify(a);
+            emit(42, j.length);}
+    ">>,
+    Proc = couch_query_servers:get_os_process(<<"javascript">>),
+    true = couch_query_servers:proc_prompt(Proc, [<<"reset">>]),
+    true = couch_query_servers:proc_prompt(Proc, [<<"add_fun">>, Src]),
+    Doc = {[]},
+    try
+        couch_query_servers:proc_prompt(Proc, [<<"map_doc">>, Doc])
+    catch
+        % Expect either an internal error thrown if it catches it and
+        % emits an error log before dying
+        throw:{<<"InternalError">>, _} ->
+            ok;
+        % It may fail and just exit the process. That's expected as well
+        throw:{os_process_error, _} ->
+            ok
+    end,
+    % Expect the process to be dead
+    ?assertThrow({os_process_error, _}, couch_query_servers:proc_prompt(Proc, [<<"reset">>])).
+
 trigger_oom(Proc) ->
     Status =
         try