Skip to content

Conversation

@harp-intel
Copy link
Contributor

This pull request improves the reliability and robustness of script execution and signal handling in both the script controller and workflow signal management. The main focus is on ensuring scripts and their child processes are properly finalized and terminated, handling edge cases around process cleanup, and improving the safety of signal delivery, especially when working with remote targets.

Process management and script finalization improvements:

  • Improved the kill_script function in internal/script/script.go to wait up to 5 seconds for a script to clean up after receiving SIGINT, only sending SIGKILL if the process is still alive, ensuring child scripts have time to finalize.
  • Added a _finalize flag in internal/script/scripts.go to prevent double-finalization and changed the trap to trigger on EXIT as well as INT and TERM, ensuring profiling data is always finalized even if the script exits unexpectedly. [1] [2]
  • Updated the wait commands after stopping profiling processes to not fail if the process is already exited, increasing script robustness.

Sequential script execution and signal handling:

  • Introduced current_seq_script to track the currently running sequential script, ensuring correct identification and termination during signal handling. Updated the signal handler to use this variable when killing the current script. [1] [2] [3] [4]

Workflow signal handling and remote target reliability:

  • Increased the wait time for process termination in signalProcessOnTarget and added a delay before signaling local child processes, reducing the risk of losing controller output when working with remote targets. Improved logic to check for the existence of PID files and processes, and enhanced debug logging for better traceability. [1] [2] [3] [4]

These changes collectively make the script execution and workflow management more robust, especially in scenarios involving remote targets or unexpected script exits.

Signed-off-by: Harper, Jason M <jason.m.harper@intel.com>
@harp-intel harp-intel requested a review from Copilot January 8, 2026 17:13
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enhances the reliability of script execution and signal handling by improving process termination logic, preventing double-finalization of scripts, and addressing race conditions in remote target communication.

Key Changes:

  • Modified script termination to wait up to 5 seconds for graceful cleanup before force-killing processes
  • Added finalization safeguards to prevent duplicate cleanup attempts and ensure profiling data is always captured
  • Improved signal handling timing and process tracking to prevent output loss when working with remote targets

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
internal/script/script.go Enhanced kill_script function with graceful termination period and added current_seq_script tracking for more reliable sequential script management
internal/script/scripts.go Added _finalize flag to prevent double-finalization, updated trap to include EXIT signal, and modified wait commands to handle already-exited processes
internal/workflow/signals.go Increased signal delivery timeout, added delay before signaling local children, improved error handling with early continues, and enhanced debug logging

Signed-off-by: Harper, Jason M <jason.m.harper@intel.com>
Signed-off-by: Harper, Jason M <jason.m.harper@intel.com>
Signed-off-by: Harper, Jason M <jason.m.harper@intel.com>
Signed-off-by: Harper, Jason M <jason.m.harper@intel.com>
@harp-intel harp-intel merged commit 4eea4b9 into main Jan 8, 2026
5 checks passed
@harp-intel harp-intel deleted the fix-remote-telemetry branch January 8, 2026 23:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants