From 4cce91ccef290551cb2cc9aae3628f395e0f7b50 Mon Sep 17 00:00:00 2001 From: Jason Thomas Date: Mon, 12 Oct 2026 17:35:13 -0600 Subject: [PATCH 1/3] fix(script runner): stop goto from the debug prompt deadlocking - Route goto called off the run thread through execute_while_paused in Ruby and Python, since the debug prompt runs on the thread that processes go/step/stop - Add RunningScript.run_thread? to detect the run thread in Ruby - Add Ruby specs and a Python test covering both threads Co-Authored-By: Claude Opus 5.5 --- .../spec/models/running_script_spec.rb | 34 +++++++ openc3/lib/openc3/utilities/running_script.rb | 17 +++- .../python/openc3/utilities/running_script.py | 27 +++--- .../test/utilities/test_running_script.py | 94 +++++++++++++++++++ 4 files changed, 158 insertions(+), 14 deletions(-) create mode 100644 openc3/python/test/utilities/test_running_script.py diff --git a/openc3-cosmos-script-runner-api/spec/models/running_script_spec.rb b/openc3-cosmos-script-runner-api/spec/models/running_script_spec.rb index 296794ced2..ea46c61d9b 100644 --- a/openc3-cosmos-script-runner-api/spec/models/running_script_spec.rb +++ b/openc3-cosmos-script-runner-api/spec/models/running_script_spec.rb @@ -446,6 +446,40 @@ end end + describe "goto" do + let(:script_api) { Object.new.extend(OpenC3::Script) } + + after do + RunningScript.class_variable_set(:@@run_thread, nil) + end + + it "hands off to the run thread when called from another thread" do + # The debug prompt evals goto on the redis subscriber thread. Running the + # script there deadlocks at the next pause because that thread is the one + # that processes go/step/stop, so goto must defer to the run thread. + running_script = RunningScript.new(script_status) + script_status.state = "breakpoint" + RunningScript.class_variable_set(:@@run_thread, Thread.new {}) + expect(script_api).not_to receive(:start) + + script_api.send(:goto, 9) + expect(running_script.execute_while_paused_info).to eq({ filename: "test_script.rb", line_no: 9, end_line_no: nil }) + + script_api.send(:goto, "other.rb", 3) + expect(running_script.execute_while_paused_info).to eq({ filename: "other.rb", line_no: 3, end_line_no: nil }) + end + + it "runs the script directly when called from the run thread" do + RunningScript.new(script_status) + RunningScript.class_variable_set(:@@run_thread, Thread.current) + expect(script_api).to receive(:start).with("test_script.rb", line_no: 9, bind_variables: true, complete: true) + script_api.send(:goto, 9) + + expect(script_api).to receive(:start).with("other.rb", line_no: 3, bind_variables: true, complete: true) + script_api.send(:goto, "other.rb", 3) + end + end + describe "breakpoint management" do it "sets a breakpoint" do RunningScript.set_breakpoint("test.rb", 10) diff --git a/openc3/lib/openc3/utilities/running_script.rb b/openc3/lib/openc3/utilities/running_script.rb index 3bec50a889..0ca82cd6ba 100644 --- a/openc3/lib/openc3/utilities/running_script.rb +++ b/openc3/lib/openc3/utilities/running_script.rb @@ -275,10 +275,19 @@ def start(procedure_name, line_no: 1, end_line_no: nil, bind_variables: false, c def goto(line_no_or_procedure_name, line_no = nil) if line_no.nil? - start(RunningScript.instance.current_filename, line_no: line_no_or_procedure_name, bind_variables: true, complete: true) + procedure_name = RunningScript.instance.current_filename + line_no = line_no_or_procedure_name else - start(line_no_or_procedure_name, line_no: line_no, bind_variables: true, complete: true) + procedure_name = line_no_or_procedure_name end + # goto typed at the debug prompt is evaluated on the thread that processes + # go/step/stop messages. Running the script there deadlocks at the next pause, + # so hand it to the run thread the same way the editor's goto does. + unless RunningScript.run_thread? + RunningScript.instance.execute_while_paused(procedure_name, line_no) + return + end + start(procedure_name, line_no: line_no, bind_variables: true, complete: true) end # Require an additional ruby file @@ -797,6 +806,10 @@ def self.instance=(value) @@instance = value end + def self.run_thread? + Thread.current == @@run_thread + end + def self.line_delay @@line_delay end diff --git a/openc3/python/openc3/utilities/running_script.py b/openc3/python/openc3/utilities/running_script.py index 6b2aa914d7..93d50d3dbf 100644 --- a/openc3/python/openc3/utilities/running_script.py +++ b/openc3/python/openc3/utilities/running_script.py @@ -1481,19 +1481,22 @@ def start(procedure_name, line_no=1, end_line_no=None, bind_variables=False, com def goto(line_no_or_procedure_name, line_no=None): if line_no is None: - start( - RunningScript.instance.current_filename(), - line_no=line_no_or_procedure_name, - bind_variables=True, - complete=True, - ) + procedure_name = RunningScript.instance.current_filename() + line_no = line_no_or_procedure_name else: - start( - line_no_or_procedure_name, - line_no=line_no, - bind_variables=True, - complete=True, - ) + procedure_name = line_no_or_procedure_name + # goto typed at the debug prompt is executed on the thread that processes + # go/step/stop messages. Running the script there deadlocks at the next pause, + # so hand it to the run thread the same way the editor's goto does. + if threading.current_thread() is not RunningScript.run_thread: + RunningScript.instance.execute_while_paused(procedure_name, line_no) + return + start( + procedure_name, + line_no=line_no, + bind_variables=True, + complete=True, + ) openc3.script.goto = goto diff --git a/openc3/python/test/utilities/test_running_script.py b/openc3/python/test/utilities/test_running_script.py new file mode 100644 index 0000000000..c49c1d397c --- /dev/null +++ b/openc3/python/test/utilities/test_running_script.py @@ -0,0 +1,94 @@ +# Copyright 2026 OpenC3, Inc. +# All Rights Reserved. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. +# See LICENSE.md for more details. + +# This file may also be used under the terms of a commercial license +# if purchased from OpenC3, Inc. + +import importlib +import sys +import threading +import unittest +from unittest.mock import MagicMock, patch + +import openc3.script +import openc3.script.api_shared +import openc3.utilities + + +running_script = None +RunningScript = None + + +class TestGoto(unittest.TestCase): + @classmethod + def setUpClass(cls): + # Importing running_script replaces functions on openc3.script and + # openc3.script.api_shared for the whole process, which breaks other tests. + # Import it here and put those modules back when we're done. + global running_script, RunningScript + cls.saved_script = dict(vars(openc3.script)) + cls.saved_api_shared = dict(vars(openc3.script.api_shared)) + running_script = importlib.import_module("openc3.utilities.running_script") + RunningScript = running_script.RunningScript + + @classmethod + def tearDownClass(cls): + for module, saved in ( + (openc3.script, cls.saved_script), + (openc3.script.api_shared, cls.saved_api_shared), + ): + vars(module).clear() + vars(module).update(saved) + sys.modules.pop("openc3.utilities.running_script", None) + if hasattr(openc3.utilities, "running_script"): + delattr(openc3.utilities, "running_script") + + def setUp(self): + # Skip __init__ since it redirects stdout/stderr and talks to Redis + self.running_script = RunningScript.__new__(RunningScript) + self.running_script.script_status = MagicMock(state="breakpoint", current_filename="test_script.py") + self.running_script.execute_while_paused_info = None + RunningScript.instance = self.running_script + start_patcher = patch.object(running_script, "start") + self.mock_start = start_patcher.start() + self.addCleanup(start_patcher.stop) + + def tearDown(self): + RunningScript.instance = None + RunningScript.run_thread = None + + def test_goto_off_run_thread_hands_off_to_run_thread(self): + mock_start = self.mock_start + # The debug prompt execs goto on the redis subscriber thread. Running the + # script there deadlocks at the next pause because that thread is the one + # that processes go/step/stop, so goto must defer to the run thread. + RunningScript.run_thread = threading.Thread(target=lambda: None) + + running_script.goto(9) + self.assertEqual( + self.running_script.execute_while_paused_info, + {"filename": "test_script.py", "line_no": 9, "end_line_no": None}, + ) + + running_script.goto("other.py", 3) + self.assertEqual( + self.running_script.execute_while_paused_info, + {"filename": "other.py", "line_no": 3, "end_line_no": None}, + ) + mock_start.assert_not_called() + + def test_goto_on_run_thread_starts_directly(self): + mock_start = self.mock_start + RunningScript.run_thread = threading.current_thread() + + running_script.goto(9) + mock_start.assert_called_with("test_script.py", line_no=9, bind_variables=True, complete=True) + + running_script.goto("other.py", 3) + mock_start.assert_called_with("other.py", line_no=3, bind_variables=True, complete=True) + self.assertIsNone(self.running_script.execute_while_paused_info) From b0224e9cd9bfc6b887402ba0bdbce65fdc28298b Mon Sep 17 00:00:00 2001 From: Jason Thomas Date: Thu, 15 Oct 2026 14:40:34 -0600 Subject: [PATCH 2/3] Fix sonarqube --- openc3/python/test/utilities/test_running_script.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/openc3/python/test/utilities/test_running_script.py b/openc3/python/test/utilities/test_running_script.py index c49c1d397c..23286f13b0 100644 --- a/openc3/python/test/utilities/test_running_script.py +++ b/openc3/python/test/utilities/test_running_script.py @@ -60,14 +60,15 @@ def setUp(self): def tearDown(self): RunningScript.instance = None - RunningScript.run_thread = None def test_goto_off_run_thread_hands_off_to_run_thread(self): mock_start = self.mock_start # The debug prompt execs goto on the redis subscriber thread. Running the # script there deadlocks at the next pause because that thread is the one # that processes go/step/stop, so goto must defer to the run thread. - RunningScript.run_thread = threading.Thread(target=lambda: None) + patcher = patch.object(RunningScript, "run_thread", threading.Thread(target=lambda: None)) + patcher.start() + self.addCleanup(patcher.stop) running_script.goto(9) self.assertEqual( @@ -84,7 +85,9 @@ def test_goto_off_run_thread_hands_off_to_run_thread(self): def test_goto_on_run_thread_starts_directly(self): mock_start = self.mock_start - RunningScript.run_thread = threading.current_thread() + patcher = patch.object(RunningScript, "run_thread", threading.current_thread()) + patcher.start() + self.addCleanup(patcher.stop) running_script.goto(9) mock_start.assert_called_with("test_script.py", line_no=9, bind_variables=True, complete=True) From efd580be96a09400274f4e02ff47bf5629d942cf Mon Sep 17 00:00:00 2001 From: Jason Thomas Date: Fri, 16 Oct 2026 07:38:48 -0600 Subject: [PATCH 3/3] fix(script runner): keep script variables for cross-file debug goto A goto typed at the debug prompt is queued for the run thread, which only bound the paused script's variables when the target was the same file. Pass an explicit binding so it matches a direct goto call. Co-Authored-By: Claude Opus 5.5 --- .../spec/models/running_script_spec.rb | 23 +++++++++++++++++-- openc3/lib/openc3/utilities/running_script.rb | 14 +++++------ .../python/openc3/utilities/running_script.py | 14 ++++++----- .../test/utilities/test_running_script.py | 19 +++++++++++++-- 4 files changed, 53 insertions(+), 17 deletions(-) diff --git a/openc3-cosmos-script-runner-api/spec/models/running_script_spec.rb b/openc3-cosmos-script-runner-api/spec/models/running_script_spec.rb index ea46c61d9b..139cf46b0f 100644 --- a/openc3-cosmos-script-runner-api/spec/models/running_script_spec.rb +++ b/openc3-cosmos-script-runner-api/spec/models/running_script_spec.rb @@ -463,10 +463,29 @@ expect(script_api).not_to receive(:start) script_api.send(:goto, 9) - expect(running_script.execute_while_paused_info).to eq({ filename: "test_script.rb", line_no: 9, end_line_no: nil }) + expect(running_script.execute_while_paused_info).to eq({ filename: "test_script.rb", line_no: 9, end_line_no: nil, bind_variables: true }) script_api.send(:goto, "other.rb", 3) - expect(running_script.execute_while_paused_info).to eq({ filename: "other.rb", line_no: 3, end_line_no: nil }) + expect(running_script.execute_while_paused_info).to eq({ filename: "other.rb", line_no: 3, end_line_no: nil, bind_variables: true }) + end + + it "keeps the script binding when the queued goto targets another file" do + running_script = RunningScript.new(script_status) + script_status.state = "breakpoint" + RunningScript.class_variable_set(:@@run_thread, Thread.new {}) + script_api.send(:goto, "other.rb", 3) + + expect(running_script).to receive(:start).with("other.rb", line_no: 3, bind_variables: true, complete: true) + running_script.check_execute_while_paused + end + + it "binds the editor's goto only when it targets the current file" do + running_script = RunningScript.new(script_status) + script_status.state = "breakpoint" + running_script.execute_while_paused("other.rb", 3) + + expect(running_script).to receive(:start).with("other.rb", line_no: 3, bind_variables: false, complete: true) + running_script.check_execute_while_paused end it "runs the script directly when called from the run thread" do diff --git a/openc3/lib/openc3/utilities/running_script.rb b/openc3/lib/openc3/utilities/running_script.rb index 0ca82cd6ba..a4fba9a623 100644 --- a/openc3/lib/openc3/utilities/running_script.rb +++ b/openc3/lib/openc3/utilities/running_script.rb @@ -284,7 +284,7 @@ def goto(line_no_or_procedure_name, line_no = nil) # go/step/stop messages. Running the script there deadlocks at the next pause, # so hand it to the run thread the same way the editor's goto does. unless RunningScript.run_thread? - RunningScript.instance.execute_while_paused(procedure_name, line_no) + RunningScript.instance.execute_while_paused(procedure_name, line_no, nil, true) return end start(procedure_name, line_no: line_no, bind_variables: true, complete: true) @@ -1116,9 +1116,10 @@ def current_backtrace trace end - def execute_while_paused(filename, line_no = 1, end_line_no = nil) + # bind_variables of nil binds only when filename is the currently running file + def execute_while_paused(filename, line_no = 1, end_line_no = nil, bind_variables = nil) if @script_status.state == 'paused' or @script_status.state == 'error' or @script_status.state == 'breakpoint' - @execute_while_paused_info = { filename: filename, line_no: line_no, end_line_no: end_line_no } + @execute_while_paused_info = { filename: filename, line_no: line_no, end_line_no: end_line_no, bind_variables: bind_variables } else scriptrunner_puts("Cannot execute selection or goto unless script is paused, breakpoint, or in error state") end @@ -1230,10 +1231,9 @@ def wait_for_go_or_stop_or_retry(error = nil) def check_execute_while_paused if @execute_while_paused_info - if @script_status.current_filename == @execute_while_paused_info[:filename] - bind_variables = true - else - bind_variables = false + bind_variables = @execute_while_paused_info[:bind_variables] + if bind_variables.nil? + bind_variables = (@script_status.current_filename == @execute_while_paused_info[:filename]) end if @execute_while_paused_info[:end_line_no] # Execute Selection While Paused diff --git a/openc3/python/openc3/utilities/running_script.py b/openc3/python/openc3/utilities/running_script.py index 93d50d3dbf..dd2e9a4b00 100644 --- a/openc3/python/openc3/utilities/running_script.py +++ b/openc3/python/openc3/utilities/running_script.py @@ -702,7 +702,8 @@ def current_backtrace(self): trace.append(f"{filename}:{lineno}:{name}:{line}") return trace - def execute_while_paused(self, filename, line_no=1, end_line_no=None): + # bind_variables of None binds only when filename is the currently running file + def execute_while_paused(self, filename, line_no=1, end_line_no=None, bind_variables=None): if ( self.script_status.state == "paused" or self.script_status.state == "error" @@ -712,6 +713,7 @@ def execute_while_paused(self, filename, line_no=1, end_line_no=None): "filename": filename, "line_no": line_no, "end_line_no": end_line_no, + "bind_variables": bind_variables, } else: self.scriptrunner_puts( @@ -864,10 +866,10 @@ def wait_for_go_or_stop_or_retry(self, error=None): def check_execute_while_paused(self): try: if self.execute_while_paused_info is not None: - if self.script_status.current_filename == self.execute_while_paused_info.get("filename"): - bind_variables = True - else: - bind_variables = False + filename = self.execute_while_paused_info.get("filename") + bind_variables = self.execute_while_paused_info.get("bind_variables") + if bind_variables is None: + bind_variables = self.script_status.current_filename == filename if self.execute_while_paused_info.get("end_line_no"): # Execute Selection While Paused state = self.script_status.state @@ -1489,7 +1491,7 @@ def goto(line_no_or_procedure_name, line_no=None): # go/step/stop messages. Running the script there deadlocks at the next pause, # so hand it to the run thread the same way the editor's goto does. if threading.current_thread() is not RunningScript.run_thread: - RunningScript.instance.execute_while_paused(procedure_name, line_no) + RunningScript.instance.execute_while_paused(procedure_name, line_no, bind_variables=True) return start( procedure_name, diff --git a/openc3/python/test/utilities/test_running_script.py b/openc3/python/test/utilities/test_running_script.py index 23286f13b0..bdb7e77760 100644 --- a/openc3/python/test/utilities/test_running_script.py +++ b/openc3/python/test/utilities/test_running_script.py @@ -73,16 +73,31 @@ def test_goto_off_run_thread_hands_off_to_run_thread(self): running_script.goto(9) self.assertEqual( self.running_script.execute_while_paused_info, - {"filename": "test_script.py", "line_no": 9, "end_line_no": None}, + {"filename": "test_script.py", "line_no": 9, "end_line_no": None, "bind_variables": True}, ) running_script.goto("other.py", 3) self.assertEqual( self.running_script.execute_while_paused_info, - {"filename": "other.py", "line_no": 3, "end_line_no": None}, + {"filename": "other.py", "line_no": 3, "end_line_no": None, "bind_variables": True}, ) mock_start.assert_not_called() + def test_queued_goto_to_another_file_keeps_binding(self): + patcher = patch.object(RunningScript, "run_thread", threading.Thread(target=lambda: None)) + patcher.start() + self.addCleanup(patcher.stop) + running_script.goto("other.py", 3) + + self.running_script.check_execute_while_paused() + self.mock_start.assert_called_with("other.py", line_no=3, bind_variables=True, complete=True) + + def test_editor_goto_binds_only_for_current_file(self): + self.running_script.execute_while_paused("other.py", 3) + + self.running_script.check_execute_while_paused() + self.mock_start.assert_called_with("other.py", line_no=3, bind_variables=False, complete=True) + def test_goto_on_run_thread_starts_directly(self): mock_start = self.mock_start patcher = patch.object(RunningScript, "run_thread", threading.current_thread())