Skip to content

Conversation

@ShoroukRamzy
Copy link
Contributor

This PR covers this issue created here #7

This change implements a two-phase graceful shutdown sequence for the FEO framework, as specified in feo/Design/shutdown_sequence.md. The primary agent now handles OS termination signals (e.g., SIGINT from Ctrl-C) to initiate a clean exit.

The shutdown process is handled for all supported signalling mechanisms:

  • signalling_direct_mpsc
  • signalling_direct_tcp
  • signalling_direct_unix
  • signalling_relayed_tcp
  • signalling_relayed_unix

Phase 1: Activity Shutdown
The scheduler sends a Shutdown signal to all running activities and waits for an acknowledgement before proceeding.

Phase 2: Agent Termination
The scheduler broadcasts a Terminate signal to all connected agents (workers and recorders if any), waits for a TerminateAck, and then exits.

Note: To verify it you can test the minia-adas example and terminate primary using Ctrl-C signal

@github-actions
Copy link

github-actions bot commented Nov 6, 2025

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run //:license-check

Status: ⚠️ Needs Review

Click to expand output
[License Check Output]
Starting local Bazel server (8.3.0) and connecting to it...
INFO: Invocation ID: e61e6264-e3ef-4268-8397-54c861980389
Computing main repo mapping: 
Computing main repo mapping: 
DEBUG: Rule 'rules_boost+' indicated that a canonical reproducible form can be obtained by modifying arguments integrity = "sha256-iKHWUIBrUN/fFOCltkTmHfDcKw0h4ZVmP7NVSoc8zBs="
DEBUG: Repository rules_boost+ instantiated at:
  <builtin>: in <toplevel>
Repository rule http_archive defined at:
  /home/runner/.bazel/external/bazel_tools/tools/build_defs/repo/http.bzl:394:31: in <toplevel>
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Loading: 
Loading: 4 packages loaded
Loading: 4 packages loaded
    currently loading: 
Analyzing: target //:license-check (5 packages loaded, 0 targets configured)
Analyzing: target //:license-check (5 packages loaded, 0 targets configured)

Analyzing: target //:license-check (82 packages loaded, 9 targets configured)

Analyzing: target //:license-check (146 packages loaded, 2545 targets configured)

Analyzing: target //:license-check (152 packages loaded, 6705 targets configured)

Analyzing: target //:license-check (152 packages loaded, 6705 targets configured)

INFO: Analyzed target //:license-check (155 packages loaded, 8721 targets configured).
[11 / 13] [Prepa] Building license.check.license_check.jar ()
INFO: From Generating Dash formatted dependency file ...:
INFO: Successfully converted 2 packages from Cargo.lock to bazel-out/k8-fastbuild/bin/formatted.txt
INFO: Found 1 target...
Target //:license.check.license_check up-to-date:
  bazel-bin/license.check.license_check
  bazel-bin/license.check.license_check.jar
INFO: Elapsed time: 19.781s, Critical Path: 0.40s
INFO: 13 processes: 4 disk cache hit, 9 internal.
INFO: Build completed successfully, 13 total actions
INFO: Running command line: bazel-bin/license.check.license_check ./formatted.txt <args omitted>
usage: org.eclipse.dash.licenses.cli.Main [-batch <int>] [-cd <url>]
       [-confidence <int>] [-ef <url>] [-excludeSources <sources>] [-help] [-lic
       <url>] [-project <shortname>] [-repo <url>] [-review] [-summary <file>]
       [-timeout <seconds>] [-token <token>]

@github-actions
Copy link

github-actions bot commented Nov 6, 2025

The created documentation from the pull request is available at: docu-html

@artemsheinacn
Copy link
Contributor

Hi @ShoroukRamzy, thanks for the PR!
Our implementation of the same feature was just merged as part of #10. Your PR supersedes it, so I think I'll prepare a revert PR to make merging easier for you.
Also please make sure to add support to bazel build configuration. The easiest way to check for bazel support is to do something like this:

set -e

SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )

buildifier -r $SCRIPT_DIR
bazelisk clean --expunge
bazelisk mod tidy
bazelisk build //:format.check
bazelisk build //:format.fix
cargo fmt --all
bazelisk build --config=lint-rust //...
bazelisk build //...

And make sure everything works. You'll also need an additional PR for score-crates to add ctrlc crate there.

@ShoroukRamzy ShoroukRamzy force-pushed the shutdown_and_termination_sig_handling branch from 9f85924 to bc86d9f Compare November 6, 2025 15:37
@artemsheinacn
Copy link
Contributor

I prepared a PR to revert our implementation #13
Before merging it, please make sure to discuss this change with @armin-acn

I would advise against rebasing on top of main branch; we better revert the original PR first and then rebase to avoid conflicts and/or previous implementation leftovers.

@ShoroukRamzy
Copy link
Contributor Author

I prepared a PR to revert our implementation #13 Before merging it, please make sure to discuss this change with @armin-acn
I would advise against rebasing on top of main branch; we better revert the original PR first and then rebase to avoid conflicts and/or previous implementation leftovers.

Hi @artemsheinacn, Thank you so much for your help. I fully agree with you. Unfortunately I did not see this comment before rebase but I will revert my changes and wait for your revert to be able to rebase cleanly.
Please note: We are not working tomorrow in Cairo, Egypt. I will resume my work next Sunday.

@artemsheinacn
Copy link
Contributor

@ShoroukRamzy by next Sunday you mean the 9th of November, right?

@ShoroukRamzy
Copy link
Contributor Author

@ShoroukRamzy by next Sunday you mean the 9th of November, right?

Yes, exactly!

@artemsheinacn
Copy link
Contributor

I'll be on a vacation next week, but @armin-acn will step up to help you merge your changes, have a nice holiday then!

@ShoroukRamzy
Copy link
Contributor Author

I'll be on a vacation next week, but @armin-acn will step up to help you merge your changes, have a nice holiday then!

Thank you for your support. Have a nice vacation in advance!

@armin-acn
Copy link
Contributor

Hi @ShoroukRamzy ,
I saw you have abandoned your PR.

Do you want me to apply our revert immediately or do you first want to work on your PR (reverting our PR locally)?

@ShoroukRamzy
Copy link
Contributor Author

Hi @ShoroukRamzy , I saw you have abandoned your PR.

Do you want me to apply our revert immediately or do you first want to work on your PR (reverting our PR locally)?

Hi @armin-acn,

I observe that you have introduced two additional commits; I kindly request that you revert before I re-open my pull request.

@armin-acn
Copy link
Contributor

Hi @ShoroukRamzy, I have reverted our commit regarding FEO termination.

@ShoroukRamzy
Copy link
Contributor Author

ShoroukRamzy commented Nov 11, 2025

Hi @ShoroukRamzy, I have reverted our commit regarding FEO termination.

Hi @armin-acn, Thank you for your help. I encountered a build issue with Ctrlc after rebasing, and I will re-open my PR once it's resolved.

@ShoroukRamzy
Copy link
Contributor Author

Hi @armin-acn,

I have an issue building after rebasing with crtlc
image

it was building correctly before rebasing, Could you please inform me if there are any limitations regarding the use of external crates, or suggest potential causes for this issue? Thanks!

@armin-acn
Copy link
Contributor

Hi Shorouk, sorry I also cannot reopen the pull request. I think it is because you rebased your branch. It seems you have to open a new one.

@ShoroukRamzy
Copy link
Contributor Author

Hi Shorouk, sorry I also cannot reopen the pull request. I think it is because you rebased your branch. It seems you have to open a new one.

Hi @armin-acn, I opened this one #15. Thanks!

@ShoroukRamzy
Copy link
Contributor Author

Hi @ShoroukRamzy, thanks for the PR! Our implementation of the same feature was just merged as part of #10. Your PR supersedes it, so I think I'll prepare a revert PR to make merging easier for you. Also please make sure to add support to bazel build configuration. The easiest way to check for bazel support is to do something like this:

set -e

SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )

buildifier -r $SCRIPT_DIR
bazelisk clean --expunge
bazelisk mod tidy
bazelisk build //:format.check
bazelisk build //:format.fix
cargo fmt --all
bazelisk build --config=lint-rust //...
bazelisk build //...

And make sure everything works. You'll also need an additional PR for score-crates to add ctrlc crate there.

Hi @armin-acn, Could this be the source of the Ctrlc build issue?

@armin-acn
Copy link
Contributor

Yes, I think it is. See my comment in your new PR.

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.

3 participants