Scripting Engine Debugger Support (#1701)

This PR introduces the support for implementing remote debuggers in
scripting engines modules.

The module API is extended with scripting engines callbacks and new
functions that can be used by scripting engine modules to implement a
remote debugger.

Most of the code that was used to implement the Lua debugger, was
refactored and moved to the `scripting_engine.c` file, and only the code
specific to the Lua engine, remained in the `debug_lua.c` file.

The `SCRIPT DEBUG (YES|NO|SYNC)` command was extend with an optional
parameter that can be used to specify the engine name, where we want to
enable the debugger. If no engine name is specified, the Lua engine is
used to keep backwards compatibility.

In
[src/valkeymodule.h](https://github.com/valkey-io/valkey/pull/1701/files#diff-b91520205c29a3a5a940786e509b2f13a5e73a1ac2016be773e62ea64c7efb28)
we see the module API changes. And in the `helloscripting.c` file we can
see how to implement a simple debugger for the dummy HELLO scripting
engine.

---------

Signed-off-by: Ricardo Dias <[email protected]>

Author: rjd15372

src/commands.def

@@ -5969,6 +5969,7 @@ struct COMMAND_ARG SCRIPT_DEBUG_mode_Subargs[] = {
 /* SCRIPT DEBUG argument table */
 struct COMMAND_ARG SCRIPT_DEBUG_Args[] = {
 {MAKE_ARG("mode",ARG_TYPE_ONEOF,-1,NULL,NULL,NULL,CMD_ARG_NONE,3,NULL),.subargs=SCRIPT_DEBUG_mode_Subargs},
+{MAKE_ARG("engine",ARG_TYPE_STRING,-1,NULL,NULL,"9.1.0",CMD_ARG_OPTIONAL,0,NULL)},
 };
 
 /********** SCRIPT EXISTS ********************/
@@ -6115,7 +6116,7 @@ struct COMMAND_ARG SCRIPT_SHOW_Args[] = {
 
 /* SCRIPT command table */
 struct COMMAND_STRUCT SCRIPT_Subcommands[] = {
-{MAKE_CMD("debug","Sets the debug mode of server-side Lua scripts.","O(1)","3.2.0",CMD_DOC_NONE,NULL,NULL,"scripting",COMMAND_GROUP_SCRIPTING,SCRIPT_DEBUG_History,0,SCRIPT_DEBUG_Tips,0,scriptCommand,3,CMD_NOSCRIPT,ACL_CATEGORY_SCRIPTING,SCRIPT_DEBUG_Keyspecs,0,NULL,1),.args=SCRIPT_DEBUG_Args},
+{MAKE_CMD("debug","Sets the debug mode of server-side Lua scripts.","O(1)","3.2.0",CMD_DOC_NONE,NULL,NULL,"scripting",COMMAND_GROUP_SCRIPTING,SCRIPT_DEBUG_History,0,SCRIPT_DEBUG_Tips,0,scriptCommand,-3,CMD_NOSCRIPT,ACL_CATEGORY_SCRIPTING,SCRIPT_DEBUG_Keyspecs,0,NULL,2),.args=SCRIPT_DEBUG_Args},
 {MAKE_CMD("exists","Determines whether server-side Lua scripts exist in the script cache.","O(N) with N being the number of scripts to check (so checking a single script is an O(1) operation).","2.6.0",CMD_DOC_NONE,NULL,NULL,"scripting",COMMAND_GROUP_SCRIPTING,SCRIPT_EXISTS_History,0,SCRIPT_EXISTS_Tips,2,scriptCommand,-3,CMD_NOSCRIPT|CMD_STALE,ACL_CATEGORY_SCRIPTING,SCRIPT_EXISTS_Keyspecs,0,NULL,1),.args=SCRIPT_EXISTS_Args},
 {MAKE_CMD("flush","Removes all server-side Lua scripts from the script cache.","O(N) with N being the number of scripts in cache","2.6.0",CMD_DOC_NONE,NULL,NULL,"scripting",COMMAND_GROUP_SCRIPTING,SCRIPT_FLUSH_History,1,SCRIPT_FLUSH_Tips,2,scriptCommand,-2,CMD_NOSCRIPT|CMD_STALE,ACL_CATEGORY_SCRIPTING,SCRIPT_FLUSH_Keyspecs,0,NULL,1),.args=SCRIPT_FLUSH_Args},
 {MAKE_CMD("help","Returns helpful text about the different subcommands.","O(1)","5.0.0",CMD_DOC_NONE,NULL,NULL,"scripting",COMMAND_GROUP_SCRIPTING,SCRIPT_HELP_History,0,SCRIPT_HELP_Tips,0,scriptCommand,2,CMD_LOADING|CMD_STALE,ACL_CATEGORY_SCRIPTING,SCRIPT_HELP_Keyspecs,0,NULL,0)},

src/commands/script-debug.json

@@ -4,7 +4,7 @@
         "complexity": "O(1)",
         "group": "scripting",
         "since": "3.2.0",
-        "arity": 3,
+        "arity": -3,
         "container": "SCRIPT",
         "function": "scriptCommand",
         "command_flags": [
@@ -34,6 +34,12 @@
                         "token": "NO"
                     }
                 ]
+            },
+            {
+                "name": "engine",
+                "type": "string",
+                "optional": true,
+                "since": "9.1.0"
             }
         ],
         "reply_schema": {

src/eval.c

@@ -49,7 +49,6 @@
 #include "monotonic.h"
 #include "resp_parser.h"
 #include "script.h"
-#include "lua/debug_lua.h"
 #include "scripting_engine.h"
 #include "sds.h"
 
@@ -561,8 +560,9 @@ void evalShaRoCommand(client *c) {
 void scriptCommand(client *c) {
     if (c->argc == 2 && !strcasecmp(c->argv[1]->ptr, "help")) {
         const char *help[] = {
-            "DEBUG (YES|SYNC|NO)",
-            "    Set the debug mode for subsequent scripts executed.",
+            "DEBUG (YES|SYNC|NO) [<engine_name>]",
+            "    Set the debug mode for subsequent scripts executed of the specified engine.",
+            "    Default engine name: 'lua'",
             "EXISTS <sha1> [<sha1> ...]",
             "    Return information about the existence of the scripts in the script cache.",
             "FLUSH [ASYNC|SYNC]",
@@ -614,19 +614,35 @@ void scriptCommand(client *c) {
         zfree(sha);
     } else if (c->argc == 2 && !strcasecmp(c->argv[1]->ptr, "kill")) {
         scriptKill(c, 1);
-    } else if (c->argc == 3 && !strcasecmp(c->argv[1]->ptr, "debug")) {
+    } else if ((c->argc == 3 || c->argc == 4) && !strcasecmp(c->argv[1]->ptr, "debug")) {
         if (clientHasPendingReplies(c)) {
             addReplyError(c, "SCRIPT DEBUG must be called outside a pipeline");
             return;
         }
+
+        const char *engine_name = c->argc == 4 ? c->argv[3]->ptr : "lua";
+        scriptingEngine *en = scriptingEngineManagerFind(engine_name);
+        if (en == NULL) {
+            addReplyErrorFormat(c, "No scripting engine found with name '%s' to enable debug", engine_name);
+            return;
+        }
+        serverAssert(en != NULL);
+
+        sds err;
         if (!strcasecmp(c->argv[2]->ptr, "no")) {
-            ldbDisable(c);
+            scriptingEngineDebuggerDisable(c);
             addReply(c, shared.ok);
         } else if (!strcasecmp(c->argv[2]->ptr, "yes")) {
-            ldbEnable(c);
+            if (scriptingEngineDebuggerEnable(c, en, &err) != C_OK) {
+                addReplyErrorSds(c, err);
+                return;
+            }
             addReply(c, shared.ok);
         } else if (!strcasecmp(c->argv[2]->ptr, "sync")) {
-            ldbEnable(c);
+            if (scriptingEngineDebuggerEnable(c, en, &err) != C_OK) {
+                addReplyErrorSds(c, err);
+                return;
+            }
             addReply(c, shared.ok);
             c->flag.lua_debug_sync = 1;
         } else {
@@ -674,11 +690,11 @@ unsigned long evalScriptsMemory(void) {
 /* Wrapper for EVAL / EVALSHA that enables debugging, and makes sure
  * that when EVAL returns, whatever happened, the session is ended. */
 void evalGenericCommandWithDebugging(client *c, int evalsha) {
-    if (ldbStartSession(c)) {
+    if (scriptingEngineDebuggerStartSession(c)) {
         evalGenericCommand(c, evalsha);
-        ldbEndSession(c);
+        scriptingEngineDebuggerEndSession(c);
     } else {
-        ldbDisable(c);
+        scriptingEngineDebuggerDisable(c);
     }
 }