Skip to content

CAMEL-25010: camel-core - Supervising route controller: do not start a route stopped during its restart attempt - #26868

Open
allthingssecurity wants to merge 1 commit into
apache:mainfrom
allthingssecurity:camel-supervising-restart-after-stop
Open

allthingssecurity wants to merge 1 commit into
apache:mainfrom
allthingssecurity:camel-supervising-restart-after-stop

Conversation

@allthingssecurity

Copy link
Copy Markdown
Contributor

Description

CAMEL-25010

The restart attempt of DefaultSupervisingRouteController only checked CamelContext.isRunAllowed() and then called doStartRoute, which takes the controller lock and starts the route. stopRoute takes the same lock and cancels the route's restart task (RouteManager.release). Cancelling only prevents future attempts: BackOffTimerTask.run() checks the status once, on entry.

So an attempt that was already running started the route after stopRoute had returned. This happens whenever the attempt waits for the controller lock during the stop, and the lock is shared by all routes, so a slow stop of any other route is enough. The route then ran without supervision (it has no restart task any more), and nothing reported it.

This change: the attempt now checks under the controller lock, before it starts the route, that:

  • its task is still the route's restart task (routes.get(r) == task);
  • the task is still active (a manual stop or start cancels it);
  • Camel is not stopping.

If any check fails, it logs that the attempt was cancelled and gives up without starting the route. RouteManager.start runs under the same lock, so a new task is always registered before its first attempt can check.

Tests: new DefaultSupervisingRouteControllerStopWhileRestartingTest. A route fails to start. Its first restart attempt is held in the RouteRestartingEvent notifier while the cause is fixed and the route is stopped manually. The attempt is then released, and the route must stay stopped. Latches and Awaitility, no sleeps. Without the fix:

AssertionFailedError: expected: <Stopped> but was: <Started>

With the fix it passes. *BackOff*,*Supervising*,*RouteController* in camel-util, camel-core and camel-management: all pass.

Found with a TLA+ model of the supervising route controller, then reproduced against the real classes, also without holding any thread: route r1 waits to restart while stopping route r2 holds the controller lock (3 of 3 runs restarted r1 after stopRoute("r1") returned). With only this change, "no restart after the user stopped the route" holds in the model with up to 3 user operations and 3 restart tasks (36k states). The same harness scenarios now leave the route stopped.

Related:

Target

  • I checked that the commit is targeting the correct branch (Camel 4 uses the main branch)

Tracking

  • If this is a large change, bug fix, or code improvement, I checked there is a JIRA issue filed for the change (usually before you start working on it).

Apache Camel coding standards and style

  • I checked that each commit in the pull request has a meaningful subject line and body.
  • I have run mvn clean install -DskipTests locally from root folder and I have committed all auto-generated changes.
    (I built and tested the affected modules, including the formatter and import-sort plugins. I did not run the full root build.)

AI-assisted contributions

  • If this PR includes AI-generated code, commits have proper co-authorship attribution (e.g., Co-authored-by trailers) and the PR description identifies the AI tool used.
    This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a Co-Authored-By trailer.

Claude Code on behalf of allthingssecurity

🤖 Generated with Claude Code

…a route stopped during its restart attempt

The restart attempt of the supervising route controller only checked
CamelContext.isRunAllowed() and then called doStartRoute, which takes
the controller lock and starts the route. stopRoute takes the same
lock and cancels the route's restart task, but cancelling only
prevents future attempts. An attempt that was already running, for
example waiting for the controller lock while another route was being
stopped, started the route after stopRoute had returned. The route was
then running without supervision, and nothing reported it.

The attempt now checks, while holding the controller lock, that its
task is still the route's restart task and still active, and that
Camel is not stopping. Otherwise it gives up without starting the
route.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@gnodet-bot gnodet-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Solid concurrency fix. The race between the restart attempt and a concurrent stopRoute is correctly closed by acquiring the controller lock before the guard check and holding it through doStartRoute. Three guard conditions (routes.get(r) != context, task status, isRunAllowed) are necessary and sufficient. Reentrant lock prevents deadlock with doStartRoute's own lock acquisition. Event notification correctly stays outside the lock to avoid lock-inversion with listeners. The test faithfully reproduces the race via RouteRestartingEvent interception with latches — no timing fragility.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

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