Add tests which are verifying comp_req__launch_man__launcher_exit_shutdown by TimoSteuerwaldETAS · Pull Request #450 · eclipse-score/lifecycle

TimoSteuerwaldETAS · 2026-08-11T11:03:20Z

Part of #317

comp_req__launch_man__launcher_exit_shutdown
The Launch Manager shall exit after performing shutdown operation by
stopping all the Processes <Process> it owns in the dependency order when requested.

github-actions · 2026-08-11T11:04:58Z

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run --lockfile_mode=error //:license-check

Status: ⚠️ Needs Review

Click to expand output

[License Check Output]
Extracting Bazel installation...
Starting local Bazel server (8.6.0) and connecting to it...
INFO: Invocation ID: b5d213cb-2572-42e6-b9e7-f03d5d5c42bd
Computing main repo mapping: 
Computing main repo mapping: 
Loading: 
Loading: 0 packages loaded
Loading: 0 packages loaded
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)

Analyzing: target //:license-check (35 packages loaded, 10 targets configured)

Analyzing: target //:license-check (89 packages loaded, 10 targets configured)

Analyzing: target //:license-check (147 packages loaded, 1130 targets configured)

Analyzing: target //:license-check (159 packages loaded, 5477 targets configured)

Analyzing: target //:license-check (169 packages loaded, 6334 targets configured)

Analyzing: target //:license-check (171 packages loaded, 7357 targets configured)

Analyzing: target //:license-check (176 packages loaded, 11463 targets configured)

Analyzing: target //:license-check (176 packages loaded, 11463 targets configured)

INFO: Analyzed target //:license-check (177 packages loaded, 11589 targets configured).
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: 24.216s, Critical Path: 0.28s
INFO: 16 processes: 4 disk cache hit, 12 internal.
INFO: Build completed successfully, 16 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 · 2026-08-13T08:26:43Z

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

These commits have been squashed together to ease review. After cherry pick some conflicts have been solved wrong, this is why there are two additional commits also. For details please also have a look onto the internal branch in etas-eng feature/create-process-launch-fit-specification-unverified The original commit message of this commit: Add a log message for SIGTERM receival Move dir tests/integration/lm_shutdown to tests/integration/lm_shutdown_during_rt_switch Update references accordingly Add lm_shutdown_during_switch_to_off A test which verifies that a launch manager shutdown signalled via SIGTERM does not cancel existent switches to off. Currently does not fail, but should fail. As switch to off gets cancelled and reinitiated by launch manager code. Let test fail, as switch to off is cancelled by lm Revert to 1s timeout for lm shutdown & add some comments Still not 100% stable. Fix cherry pick odyssey Former fix for SIGSEGV of old branch

Other ControlClientMocks need to be updated separately.

WilliamRoebuck

I have a few suggestions, although many are about the length of inline comments 😆

WilliamRoebuck · 2026-08-14T08:16:42Z

+            // Only processes with a live OS process still to stop actually consume the
+            // SIGTERM+timeout wait during the transition to Off. Already terminated/failed
+            // processes short-circuit deactivation, and never-started nodes (pid == 0) have
+            // nothing to wait for, so neither should inflate the bound. This excludes
+            // one-shot / ready-on-termination processes that have already exited.


This comment should be removed, it makes the code seem more complex than it is

WilliamRoebuck · 2026-08-14T08:19:05Z

+    /// @details The largest configured shutdown_timeout (termination_timeout_ms_) across the process
+    /// nodes of this process group that still have a live process to stop (pid > 0 and not yet
+    /// terminated), or zero if there are none.
+    /// @return The timeout in milliseconds, or zero if there are no live processes to stop.


Suggested change

/// @return The timeout in milliseconds, or zero if there are no live processes to stop.

/// @brief Returns the largest configured shutdown_timeout across all running processes

I'm not sure if it shows up my suggested change but I think the @details section should be removed

WilliamRoebuck · 2026-08-14T08:22:00Z

    /// @return The current state of this process.
    score::mw::lifecycle::ProcessState getState() const;

+    /// @return The configured shutdown_timeout (termination_timeout_ms_) for this process, or zero


Suggested change

/// @return The configured shutdown_timeout (termination_timeout_ms_) for this process, or zero

/// @return The configured shutdown_timeout for this process, or zero

WilliamRoebuck · 2026-08-14T08:29:07Z

+    // Stop and join the worker threads BEFORE destroying the process groups.
+    // Worker threads run ProcessInfoNode::doWork(), which dereferences its Graph
+    // (nodeExecuted(), getState(), ...) via a raw back-pointer. If a transition is
+    // still completing on a worker thread (e.g. an in-progress switch to Off that
+    // is allowed to continue during shutdown), destroying the graphs first would be
+    // a use-after-free.
    worker_threads_.reset();
    worker_jobs_.reset();
+
+    process_groups_.clear();


I think this might not be necessary. The situation the comment describes no longer exists and the worker threads don't have any access to Graph

WilliamRoebuck · 2026-08-14T08:29:37Z


            watchdog_->serviceWatchdog();
        }
+        LM_LOG_WARN() << "ProcessGroupManager::run() - received SIGTERM, exiting";


Should be debug level I think

I would stick at least to info level, as this is a major state change to the whole launch manager.

WilliamRoebuck · 2026-08-14T08:40:43Z

+        # Wait until the switch to run_target_c is underway: component_a is being
+        # terminated (and is now stalling), so run_target_c has not been activated
+        # yet. This state is signalled via file a_terminating.
+        # This is the window in which the shutdown request must win.
+        _wait_for_file(target, a_terminating, proc, timeout_s=10.0)
+
+        # run_target_c must not have started yet at this point.
+        exit_code, _ = target.execute(f"test -f {c_started}")
+        # The assertion below could only fail if either the sleep in component_a's termination code is too short
+        # or component_a has been killed by launch manager, because it takes too long to react on SIGTERM.
+        assert exit_code != 0, (
+            "run_target_c was activated before shutdown was requested - this should not happen"
+        )
+
+        # Request shutdown: send SIGTERM to the launch manager process only, so
+        # that the launch manager itself stops the processes it owns (rather than
+        # the OS terminating the whole process group directly).
+        lm_pid = _launch_manager_pid(target)
+        assert lm_pid is not None, "Could not find the running launch_manager process"
+        logger.info(f"Sending SIGTERM to launch_manager (pid {lm_pid})")
+        exit_code, _ = target.execute(f"kill -15 {lm_pid}")
+        assert exit_code == 0, "Failed to send SIGTERM to the launch manager"


Would it be possible to achieve the same behaviour simply using the normal run until file deployed method? This sends a sigterm to lm and lets it terminate gracefully

WilliamRoebuck · 2026-08-14T08:45:28Z

+/// @brief How long component_a stalls while it is being terminated. By stalling here, component_a keeps the switch (to
+/// the "Off" run target) in the STOP phase, giving the test a deterministic window to send SIGTERM to the launch
+/// manager while the switch to Off is still in progress.
+///
+/// It must be comfortably larger than the time the test needs to observe
+/// `a_terminating` and deliver the SIGTERM to the launch manager, so the switch
+/// to Off is still in progress when that SIGTERM arrives. It must also be
+/// comfortably smaller than component_a's configured shutdown_timeout: the launch
+/// manager honours that per-process shutdown_timeout during its own shutdown, so
+/// component_a is given time to exit on its own and terminates gracefully (and
+/// writes its XML result) rather than being force-terminated (SIGKILLed).


This can probably be 2 lines also

WilliamRoebuck · 2026-08-14T08:46:50Z

+        _wait_for_file(target, a_terminating, proc, timeout_s=10.0)
+
+        # Request shutdown: send SIGTERM to the launch manager process only, so
+        # that the launch manager itself stops the processes it owns (rather than
+        # the OS terminating the whole process group directly).
+        lm_pid = _launch_manager_pid(target)
+        assert lm_pid is not None, "Could not find the running launch_manager process"
+        logger.info(f"Sending SIGTERM to launch_manager (pid {lm_pid})")
+        exit_code, _ = target.execute(f"kill -15 {lm_pid}")
+        assert exit_code == 0, "Failed to send SIGTERM to the launch manager"


Same comment as the other python file, hopefully we can remove these two methods

WilliamRoebuck · 2026-08-14T08:52:13Z

+    ASSERT_TRUE(check_clean({test_end_location, a_started, a_terminating}));
+
+    const auto pid = getpid();
+    const std::string step_msg = "Report running with pid == " + std::to_string(pid);


Please don't rely on log messages for tests! If we use the normal shutdown procedure as I mentioned in the other test's python file, this shouldn't be needed

WilliamRoebuck · 2026-08-14T09:25:03Z

+    /// Poll the atomic flag instead of calling pause() directly. A process-directed
+    /// signal (from kill) is delivered to an arbitrary one of the process's threads
+    /// that has the signal unblocked. If the test process has more than one thread
+    /// (e.g. a ControlClient, whose constructor spawns a background IPC thread), the
+    /// handler may run on a background thread and set exitRequested there. pause()
+    /// only returns for a signal handled on the calling thread, so a main thread
+    /// blocked in pause() would not wake - it would stay parked until the launch
+    /// manager SIGKILLs it at its shutdown_timeout, and never write its XML result.
+    /// Polling observes the flag regardless of which thread handled the signal.


This doesn't sound correct to me. If the child thread caught the signal, would exit requested actually be set? I ran the test using the normal pause() 450 times and didn't get any failures. I would leave these changes out of this PR and create an issue if something pops up again

github-project-automation Bot added this to LCM - Lifecycle & Health FT Aug 11, 2026

github-project-automation Bot moved this to Backlog in LCM - Lifecycle & Health FT Aug 11, 2026

TimoSteuerwaldETAS requested a deployment to workflow-approval August 11, 2026 11:03 — with GitHub Actions Waiting

TimoSteuerwaldETAS force-pushed the feature/shutdown_tests branch from f064618 to bee9de9 Compare August 11, 2026 11:48

TimoSteuerwaldETAS requested a deployment to workflow-approval August 11, 2026 11:48 — with GitHub Actions Waiting

TimoSteuerwaldETAS requested a deployment to workflow-approval August 11, 2026 11:49 — with GitHub Actions Waiting

TimoSteuerwaldETAS requested a deployment to workflow-approval August 13, 2026 08:18 — with GitHub Actions Waiting

TimoSteuerwaldETAS requested a deployment to workflow-approval August 13, 2026 10:48 — with GitHub Actions Waiting

TimoSteuerwaldETAS requested a deployment to workflow-approval August 13, 2026 11:54 — with GitHub Actions Waiting

TimoSteuerwaldETAS force-pushed the feature/shutdown_tests branch from 66c58a4 to 3494414 Compare August 13, 2026 12:23

TimoSteuerwaldETAS requested a deployment to workflow-approval August 13, 2026 12:23 — with GitHub Actions Waiting

TimoSteuerwaldETAS marked this pull request as ready for review August 13, 2026 12:27

TimoSteuerwaldETAS requested review from FScholPer, MaciejKaszynski, NicolasFussberger, anmittag, antonkri, pawelrutkaq and ramceb as code owners August 13, 2026 12:27

TimoSteuerwaldETAS requested a review from WilliamRoebuck August 13, 2026 12:28

TimoSteuerwaldETAS added 10 commits August 13, 2026 14:53

Add crashdump support in docker environment

fb82071

Log PID

8f8de8f

Proposed fix by Claude.

54ec277

Increase shutdown_timeout

3826d82

Revert FAIL

3df77a5

Honor the termination timeout during shutdown

61a750b

Add method to TestRunner

00a9f51

Other ControlClientMocks need to be updated separately.

Only check shutdown timeout of currently active runtarget

412097d

Consider only the shutdown timeout of active processes

871daa5

Format fix

3f23834

TimoSteuerwaldETAS force-pushed the feature/shutdown_tests branch from 3494414 to 3f23834 Compare August 13, 2026 12:53

TimoSteuerwaldETAS requested a deployment to workflow-approval August 13, 2026 12:53 — with GitHub Actions Waiting

Reconstruct file paths automatically

592aed3

TimoSteuerwaldETAS requested a deployment to workflow-approval August 13, 2026 14:15 — with GitHub Actions Waiting

Align to naming schema on main branch

e270b48

TimoSteuerwaldETAS deployed to workflow-approval August 13, 2026 14:58 — with GitHub Actions Active

TimoSteuerwaldETAS changed the title ~~Add tests which are verifying comp_req__lifecycle__launcher_exit_shutdown~~ Add tests which are verifying comp_req__launch_man__launcher_exit_shutdown Aug 14, 2026

Update requirement name

c6e7006

TimoSteuerwaldETAS requested a deployment to workflow-approval August 14, 2026 08:17 — with GitHub Actions Waiting

WilliamRoebuck suggested changes Aug 14, 2026

View reviewed changes

	/// @return The timeout in milliseconds, or zero if there are no live processes to stop.
	/// @brief Returns the largest configured shutdown_timeout across all running processes

	/// @return The configured shutdown_timeout (termination_timeout_ms_) for this process, or zero
	/// @return The configured shutdown_timeout for this process, or zero

Uh oh!

Conversation

TimoSteuerwaldETAS commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

License Check Results

Uh oh!

github-actions Bot commented Aug 13, 2026

Uh oh!

WilliamRoebuck left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

TimoSteuerwaldETAS commented Aug 11, 2026 •

edited

Loading

github-actions Bot commented Aug 11, 2026 •

edited

Loading