[
https://issues.apache.org/jira/browse/CAMEL-26449?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Claus Ibsen updated CAMEL-26449:
--------------------------------
Fix Version/s: 4.24.0
> camel-core - DefaultShutdownStrategy: a route stop that times out while the
> CamelContext stops forces or cuts short the graceful shutdown of the whole
> CamelContext
> -------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: CAMEL-26449
> URL: https://issues.apache.org/jira/browse/CAMEL-26449
> Project: Camel
> Issue Type: Bug
> Components: camel-core
> Reporter: shashank
> Assignee: shashank
> Priority: Major
> Fix For: 4.24.0
>
>
> Found with a TLA+ model of {{DefaultShutdownStrategy.doShutdown}} called by
> two threads at once, and reproduced on the real classes.
> {{DefaultShutdownStrategy}} keeps the state of a shutdown in fields of the
> strategy: {{currentShutdownTaskFuture}} (the task the caller waits for and
> cancels on timeout), {{timeoutOccurred}} (also handed to the
> {{ShutdownTask}}, whose wait loop ends when it is set) and {{forceShutdown}}.
> Two shutdowns can run at the same time: a route stop or suspend
> ({{stopRoute(id, timeout, unit)}}, {{suspendRoute}}, a route policy, the
> control bus, a reload) holds the route lock of the CamelContext, while
> {{CamelContext.stop()}} ({{shutdownForced}}) runs under the lifecycle lock,
> so nothing keeps them apart. Then the timeout of one changes the other:
> * *The route stop starts first, the CamelContext stop after it.* The
> CamelContext stop overwrites {{currentShutdownTaskFuture}} with its own task.
> When the route stop times out it cancels {{currentShutdownTaskFuture}}, that
> is the task of the CamelContext stop (its own task keeps running), and sets
> {{forceShutdown}} to false. {{Future.get}} of the CamelContext stop throws
> {{CancellationException}}, which is handled as a timeout: the CamelContext
> forces every route to shut down right away ({{shutdownRoutesNow}}, forced
> {{prepareShutdown}}) after the timeout of the route stop (seconds) instead of
> its own (45 seconds by default). Consumers are stopped under in-flight
> exchanges. If the route stop clears the field ({{finally}}) between the
> assignment and the read in the CamelContext stop,
> {{currentShutdownTaskFuture.get}} throws a {{NullPointerException}}, also
> handled as a timeout.
> * *The CamelContext stop starts first, the route stop after it.* The task of
> the route stop waits behind the task of the CamelContext stop (single-thread
> executor), so the route stop times out and sets {{timeoutOccurred}}. The wait
> loop of the CamelContext's {{ShutdownTask}} reads the same flag and ends: the
> CamelContext stops all routes while their exchanges are still in flight,
> without any warning ({{Graceful shutdown ... completed}}, then {{Shutting
> down while there are still 2 inflight exchanges}}).
> Either way messages in flight on the other routes are cut off during a
> graceful shutdown (redelivered or lost, depending on the consumer). A pod
> that is stopped while a route is being stopped (a route policy, a cluster
> leadership change, the control bus, management) is the usual way to get there.
> *TLA+ model* (two callers, the single-thread executor, the shared fields;
> each caller and task step is one step of {{doShutdown}} /
> {{ShutdownTask.run}}). With the CamelContext stop never timing out,
> {{NoForeignForce}} (a caller forces its routes only when its own timeout
> elapsed) is violated:
> {noformat}
> 1 x: timeoutOccurred.set(false) (DSS:211)
> 2 x: currentShutdownTaskFuture = submit(Tx) (DSS:213)
> 3 r: timeoutOccurred.set(false); 4 r: currentShutdownTaskFuture = submit(Tr)
> 5 r: get(field) -> waits for Tr; 6 x: get(field) -> waits for Tr (re-read of
> the field, DSS:216)
> 7 r: its timeout elapses; 8 r: cancel(field) ...
> 9 x: CancellationException -> catch -> shutdownRoutesNow (forced, its timeout
> did not elapse)
> {noformat}
> and {{NoEarlyExit}} (the task of a caller stops waiting with exchanges in
> flight only when that caller timed out) is violated (a 10-state trace): the
> route stop times out, sets the shared flag, and the task of the CamelContext
> stop leaves its wait loop. When the CamelContext stop can time out,
> {{ForceStays}} is violated too (a route stop resets {{forceShutdown}} after a
> forced CamelContext stop). With the future and the flag local to each call,
> and {{forceShutdown}} only set to true, all three hold (1,126 states; 415
> when the CamelContext stop never times out); witnesses show both graceful
> completion and both callers forcing after their own timeouts are reachable.
> *Reproduced* on main with tests that use routes whose consumer is suspended
> during a graceful shutdown and an exchange in flight on each route: route a
> is stopped with a 5 second timeout while the CamelContext stops with a 60
> second timeout, and the CamelContext stop starts first and route a is stopped
> with a 1 second timeout. On main the consumer of route b is stopped while its
> exchange is in flight ({{forceShutdown}} true after 5 seconds), or the
> CamelContext stops after 2 seconds with 2 exchanges in flight and no timeout
> warning. With the change the CamelContext waits until the exchange of route b
> completes.
> A third way, found by repeating the tests: a route stop that times out while
> the CamelContext is stopping shuts its route down, and
> {{DefaultRoute.doShutdown}} clears the services list of the route while the
> task of the CamelContext stop iterates it:
> {{ConcurrentModificationException}}, and the CamelContext stops all routes
> without waiting ("Error occurred while shutting down routes. This exception
> will be ignored.").
> *Proposal*
> Keep the future and the timeout flag of a shutdown in local variables (the
> fields are still set, for {{getCurrentShutdownTaskFuture()}} and
> {{isTimeoutOccurred()}}, and the field is only cleared when it still holds
> the task of that call), only ever set {{forceShutdown}} to true, make the
> services list of a route copy-on-write, and let the timeout path use the copy
> of the route list. Code outside the strategy that reads
> {{isTimeoutOccurred()}} (camel-cxf {{CxfSpringEndpoint}},
> {{LoggingExceptionHandler}}) still sees the last writer; that is a follow-up.
> _Filed with Claude Code on behalf of allthingssecurity._
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
