You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "allthingssecurity (via GitHub)" <gi...@apache.org> on 2026/10/19 00:09:54 UTC
[PR] CAMEL-25742: camel-coap - cancel the observation when an observe consumer stops [camel]
allthingssecurity opened a new pull request, #28069:
URL: https://github.com/apache/camel/pull/28069
# Description
[CAMEL-25742](https://issues.apache.org/jira/browse/CAMEL-25742)
`CoAPObserver` (the consumer used with `observe=true`) calls `client.observe(this)` on start and drops the returned `CoapObserveRelation`; it has no `doStop`. So when its route is stopped the server keeps it as an observer and every notification is still handed to the stopped consumer (while the route is stopped it fails with `RouteStoppingException`, logged as a WARN). When the route is started again the new consumer observes a second time, and the stopped consumer routes into the started route again: each notification is then routed twice (and once more after every further stop / start). `onError` also observes again after a stop.
Change:
- keep the relation returned by `observe`;
- `doStop` cancels it with `proactiveCancel()` (Californium cancels the observation in the client at once, so no later notification reaches the consumer, and sends the deregistration to the server), then shuts the client down;
- `onError` observes again only while the consumer is running.
What users can notice: a stopped observe route no longer receives notifications, and a restarted one receives each notification once.
Tests:
- New `CoAPObserveRestartTest` (2 tests, Camel CoAP server with an observable resource in the same JVM): stop and start the observer route, change the resource once, and the notification is routed once; and stop the observer route, the resource must have no observer, start it again, one observer.
- Without the change both fail: `mock://observed Received message count. Expected: <1> but was: <2>`, and the server still holds the stopped consumer's observation (`ConditionTimeout` at the `observerCount() == 0` wait), 3 of 3 runs each.
- With the change the camel-coap tests pass: 78 run, 0 failures, 0 errors, 0 skipped.
What is not changed (possible follow-ups): the server side `CoAPConsumer` has no `doStop` either, so a stopped route keeps answering requests until a new consumer takes its path; `CoapClient.shutdown()` does not destroy the client's own `CoapEndpoint` (socket and threads).
What was not run: the full root build.
# Target
- [x] I checked that the commit is targeting the correct branch (Camel 4 uses the `main` branch)
# Tracking
- [x] If this is a large change, bug fix, or code improvement, I checked there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for the change (usually before you start working on it): [CAMEL-25742](https://issues.apache.org/jira/browse/CAMEL-25742).
# Apache Camel coding standards and style
- [x] 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 camel-coap, including the formatter and import-sort plugins. I did not run the full root build.)
# AI-assisted contributions
- [x] 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](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CAMEL-25742: camel-coap - cancel the observation when an observe consumer stops [camel]
Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus commented on code in PR #28069:
URL: https://github.com/apache/camel/pull/28069#discussion_r4310716140
##########
components/camel-coap/src/main/java/org/apache/camel/coap/CoAPObserver.java:
##########
@@ -63,13 +65,67 @@ public void onLoad(CoapResponse response) {
}
}
+ @Override
+ protected void doStop() throws Exception {
+ // cancel the observation, or the server keeps sending notifications to this consumer after it was stopped, and
+ // a route that is started again then gets every notification twice (once from the stopped consumer)
+ Observation current = observation;
+ observation = null;
+ if (current != null) {
+ current.cancel();
+ }
+ if (client != null) {
+ client.shutdown();
+ client = null;
+ }
+ super.doStop();
+ }
+
@Override
public void onError() {
getExceptionHandler().handleException(new IOException("CoAP request timed out or has been rejected by the server"));
- startObserve();
+ if (isRunAllowed()) {
+ startObserve();
+ }
}
private void startObserve() {
- this.client.observe(this);
+ Observation next = new Observation();
+ this.observation = next;
+ next.relation = this.client.observe(next);
+ }
Review Comment:
A lock-free alternative to the lock: read `client` once, and check again after the relation is set. `doStop()` runs after the status became STOPPING, and both `status` and `observation`/`relation`/`cancelled` are volatile. So either `doStop()` sees this observation with its relation set, or this check sees `!isRunAllowed()` / `cancelled` and cancels it. `proactiveCancel()` is CAS-guarded, so a double cancel is harmless.
```suggestion
private void startObserve() {
CoapClient current = this.client;
if (current == null) {
return;
}
Observation next = new Observation();
this.observation = next;
next.relation = current.observe(next);
if (next.cancelled || !isRunAllowed()) {
// doStop() ran while this observation was being set up
next.cancel();
}
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CAMEL-25742: camel-coap - cancel the observation when an observe consumer stops [camel]
Posted by "allthingssecurity (via GitHub)" <gi...@apache.org>.
allthingssecurity commented on code in PR #28069:
URL: https://github.com/apache/camel/pull/28069#discussion_r4311053151
##########
components/camel-coap/src/main/java/org/apache/camel/coap/CoAPObserver.java:
##########
@@ -63,13 +65,67 @@ public void onLoad(CoapResponse response) {
}
}
+ @Override
+ protected void doStop() throws Exception {
+ // cancel the observation, or the server keeps sending notifications to this consumer after it was stopped, and
+ // a route that is started again then gets every notification twice (once from the stopped consumer)
+ Observation current = observation;
+ observation = null;
+ if (current != null) {
+ current.cancel();
+ }
+ if (client != null) {
+ client.shutdown();
+ client = null;
+ }
+ super.doStop();
+ }
+
@Override
public void onError() {
getExceptionHandler().handleException(new IOException("CoAP request timed out or has been rejected by the server"));
- startObserve();
+ if (isRunAllowed()) {
+ startObserve();
+ }
}
private void startObserve() {
- this.client.observe(this);
+ Observation next = new Observation();
+ this.observation = next;
+ next.relation = this.client.observe(next);
+ }
Review Comment:
Thanks for this. Your reasoning holds for the cancel: with `status` set to STOPPING before `doStop()` and the volatile fields, either `doStop()` cancels the new observation with its relation set, or the re-check sees that the consumer is stopping and cancels it.
I went with the lock in 91d6046330f1 though, because of the client side. With the re-check, `doStop()` can run completely, including `client.shutdown()`, between reading `client` and `current.observe(next)`. In Californium 3.14.0 `shutdown()` only nulls and `shutdownNow()`s the two executors, and `observe()` then calls `getSecondaryExecutor()`, which lazily creates a new `CoapClient(secondary)` executor that nothing shuts down. So the observation is cancelled, but each such race leaves an executor behind.
The new test `observerStoppedWhileObservingAgainCancelsTheNewObservation` stops the consumer from inside the observe call of the observe-again (no production hook: the endpoint returns a `CoapClient` subclass). On the previous commit it fails on the leaked observation; with the re-check instead of the lock it fails on `observed again with the client that was shut down`; with the lock it passes. `doStart`, `doStop` and the observe-again in `onError` take one `ReentrantLock` (as `CoAPComponent` / `CoAPProducer` use); a cancelled handler drops responses without taking it, so the cancel inside `doStop` cannot deadlock with a late response. camel-coap: 82 tests, 0 failures.
_Claude Code on behalf of allthingssecurity_
##########
components/camel-coap/src/main/java/org/apache/camel/coap/CoAPObserver.java:
##########
@@ -63,13 +65,67 @@ public void onLoad(CoapResponse response) {
}
}
+ @Override
+ protected void doStop() throws Exception {
+ // cancel the observation, or the server keeps sending notifications to this consumer after it was stopped, and
+ // a route that is started again then gets every notification twice (once from the stopped consumer)
+ Observation current = observation;
+ observation = null;
+ if (current != null) {
+ current.cancel();
+ }
+ if (client != null) {
+ client.shutdown();
+ client = null;
+ }
+ super.doStop();
+ }
+
@Override
public void onError() {
getExceptionHandler().handleException(new IOException("CoAP request timed out or has been rejected by the server"));
- startObserve();
+ if (isRunAllowed()) {
+ startObserve();
+ }
}
private void startObserve() {
- this.client.observe(this);
+ Observation next = new Observation();
+ this.observation = next;
+ next.relation = this.client.observe(next);
Review Comment:
Thanks, both windows are real. Closed in 91d6046330f1: `doStart`, `doStop` and the observe-again in `onError` now hold one lock (a `ReentrantLock`, so `onError` -> `startObserve` is fine), and `onError` observes again only while the consumer is running and still has its client. New test: `observerStoppedWhileObservingAgainCancelsTheNewObservation`, which fails on the previous commit (the new observation is not cancelled and the server keeps it).
_Claude Code on behalf of allthingssecurity_
##########
components/camel-coap/src/main/java/org/apache/camel/coap/CoAPObserver.java:
##########
@@ -31,6 +32,7 @@
public class CoAPObserver extends DefaultConsumer implements CoapHandler {
Review Comment:
I kept it on purpose: `CoAPObserver` is a public class and `onLoad` / `onError` are public, so removing the interface would be an API change for anyone who calls or extends them. `Observation` delegates to those methods, so they stay the one place where a response is handled. Claus is fine with either.
_Claude Code on behalf of allthingssecurity_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CAMEL-25742: camel-coap - cancel the observation when an observe consumer stops [camel]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #28069:
URL: https://github.com/apache/camel/pull/28069#discussion_r4310232874
##########
components/camel-coap/src/main/java/org/apache/camel/coap/CoAPObserver.java:
##########
@@ -31,6 +32,7 @@
public class CoAPObserver extends DefaultConsumer implements CoapHandler {
Review Comment:
⚠️ **Stale interface declaration**: `CoAPObserver` still `implements CoapHandler` but `client.observe()` now receives the inner `Observation` instance, not `this`. The outer class is no longer used as a handler — remove the interface to avoid confusion.
```suggestion
public class CoAPObserver extends DefaultConsumer {
```
##########
components/camel-coap/src/main/java/org/apache/camel/coap/CoAPObserver.java:
##########
@@ -63,13 +65,67 @@ public void onLoad(CoapResponse response) {
}
}
+ @Override
+ protected void doStop() throws Exception {
+ // cancel the observation, or the server keeps sending notifications to this consumer after it was stopped, and
+ // a route that is started again then gets every notification twice (once from the stopped consumer)
+ Observation current = observation;
+ observation = null;
+ if (current != null) {
+ current.cancel();
+ }
+ if (client != null) {
+ client.shutdown();
+ client = null;
+ }
+ super.doStop();
+ }
+
@Override
public void onError() {
getExceptionHandler().handleException(new IOException("CoAP request timed out or has been rejected by the server"));
- startObserve();
+ if (isRunAllowed()) {
+ startObserve();
+ }
}
private void startObserve() {
- this.client.observe(this);
+ Observation next = new Observation();
+ this.observation = next;
+ next.relation = this.client.observe(next);
Review Comment:
⚠️ **Race condition — server-side observer leak**: `doStop()` can interleave between `this.observation = next` (line 94) and `next.relation = this.client.observe(next)` (line 95). When it does, `cancel()` sees `relation == null`, skips `proactiveCancel()`, and the server retains the leaked observer permanently.
Additionally, `doStop()` can null `client` between `isRunAllowed()` (in `onError`) and `this.client.observe(next)` here, causing a NPE.
Both windows require a shared lock (see review body for the full fix).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CAMEL-25742: camel-coap - cancel the observation when an observe consumer stops [camel]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #28069:
URL: https://github.com/apache/camel/pull/28069#issuecomment-6260862325
<!-- ci-tested-modules -->
:test_tube: **CI tested the following changed modules:**
- `components/camel-coap`
---
<details><summary>:microscope: Scalpel shadow comparison — Scalpel: 8 of 706 tested, 27 compile-only — current: 8 all tested</summary>
[Maveniverse Scalpel](https://github.com/maveniverse/scalpel) detected **8 affected modules** (current approach: 8).
**Skip-tests mode would test 8 modules** (1 direct + 7 downstream), **skip tests for 27** (generated code, meta-modules)
<details><summary>Modules Scalpel would test (8)</summary>
- `camel-coap` ← components/camel-coap/src/main/java/org/apache/camel/coap/CoAPObserver.java, components/camel-coap/src/test/java/org/apache/camel/coap/CoAPObserveRestartTest.java
- `camel-jbang-mcp` ← downstream of org.apache.camel:camel-catalog
- `camel-jbang-plugin-mcp` ← downstream of org.apache.camel:camel-jbang-core
- `camel-jbang-plugin-tui` ← downstream of org.apache.camel:camel-catalog
- `camel-jbang-plugin-validate` ← downstream of org.apache.camel:camel-yaml-dsl-validator
- `camel-launcher-container` ← downstream of org.apache.camel:camel-launcher
- `camel-yaml-dsl-validator` ← downstream of org.apache.camel:camel-catalog
- `camel-yaml-dsl-validator-maven-plugin` ← downstream of org.apache.camel:camel-yaml-dsl-validator
</details>
<details><summary>Modules with tests skipped (27)</summary>
- `apache-camel`
- `camel-allcomponents`
- `camel-catalog`
- `camel-catalog-console`
- `camel-catalog-maven`
- `camel-catalog-suggest`
- `camel-componentdsl`
- `camel-endpointdsl`
- `camel-endpointdsl-support`
- `camel-itest`
- `camel-jbang-core`
- `camel-jbang-it`
- `camel-jbang-main`
- `camel-jbang-plugin-edit`
- `camel-jbang-plugin-generate`
- `camel-jbang-plugin-kubernetes`
- `camel-jbang-plugin-test`
- `camel-kamelet-main`
- `camel-launcher`
- `camel-report-maven-plugin`
- `camel-route-parser`
- `camel-yaml-dsl`
- `camel-yaml-dsl-deserializers`
- `camel-yaml-dsl-maven-plugin`
- `coverage`
- `docs`
- `dummy-component`
</details>
> :information_source: Shadow mode — Scalpel observes but does not affect test execution. [Learn more](https://github.com/maveniverse/scalpel)
</details>
<details><summary>All tested modules (35 modules, 2m 7s total)</summary>
**Total reactor time:** 2m 7s
| Module | Duration | Status |
| --- | --- | --- |
| Camel :: YAML DSL | 20.7s | SUCCESS |
| Camel :: Catalog :: Camel Catalog | 16.4s | SUCCESS |
| Camel :: YAML DSL :: Validator | 15.4s | SUCCESS |
| Camel :: Component DSL | 15.3s | SUCCESS |
| Camel :: Docs | 11.3s | SUCCESS |
| Camel :: Kamelet Main | 9.5s | SUCCESS |
| Camel :: Catalog :: Camel Report Maven Plugin | 7.3s | SUCCESS |
| Camel :: YAML DSL :: Deserializers | 6.3s | SUCCESS |
| Camel :: Catalog :: Camel Route Parser | 6.2s | SUCCESS |
| Camel :: All Components Sync point | 4.5s | SUCCESS |
| Camel :: YAML DSL :: Validator Maven Plugin | 3.4s | SUCCESS |
| Camel :: YAML DSL :: Maven Plugins | 2.6s | SUCCESS |
| Camel :: Catalog :: Maven | 2.4s | SUCCESS |
| Camel :: Catalog :: Suggest (deprecated) | 2.2s | SUCCESS |
| Camel :: Catalog :: Dummy Component | 1.1s | SUCCESS |
| Camel :: Catalog :: Console | 0.9s | SUCCESS |
| Camel :: Assembly | 0.7s | SUCCESS |
| Camel :: Endpoint DSL :: Support | 0.7s | SUCCESS |
| Camel :: CoAP | n/a | |
| Camel :: Coverage | n/a | |
| Camel :: Endpoint DSL | n/a | |
| Camel :: Integration Tests | n/a | |
| Camel :: JBang :: Core | n/a | |
| Camel :: JBang :: Integration tests | n/a | |
| Camel :: JBang :: MCP | n/a | |
| Camel :: JBang :: Main | n/a | |
| Camel :: JBang :: Plugin :: Edit | n/a | |
| Camel :: JBang :: Plugin :: Generate | n/a | |
| Camel :: JBang :: Plugin :: Kubernetes | n/a | |
| Camel :: JBang :: Plugin :: MCP | n/a | |
| Camel :: JBang :: Plugin :: TUI | n/a | |
| Camel :: JBang :: Plugin :: Testing | n/a | |
| Camel :: JBang :: Plugin :: Validate | n/a | |
| Camel :: Launcher | n/a | |
| Camel :: Launcher :: Container | n/a | |
**Top 20 slowest modules:**
- `Camel :: YAML DSL` (20.7s)
- `Camel :: Catalog :: Camel Catalog` (16.4s)
- `Camel :: YAML DSL :: Validator` (15.4s)
- `Camel :: Component DSL` (15.3s)
- `Camel :: Docs` (11.3s)
- `Camel :: Kamelet Main` (9.5s)
- `Camel :: Catalog :: Camel Report Maven Plugin` (7.3s)
- `Camel :: YAML DSL :: Deserializers` (6.3s)
- `Camel :: Catalog :: Camel Route Parser` (6.2s)
- `Camel :: All Components Sync point` (4.5s)
- `Camel :: YAML DSL :: Validator Maven Plugin` (3.4s)
- `Camel :: YAML DSL :: Maven Plugins` (2.6s)
- `Camel :: Catalog :: Maven` (2.4s)
- `Camel :: Catalog :: Suggest (deprecated)` (2.2s)
- `Camel :: Catalog :: Dummy Component` (1.1s)
- `Camel :: Catalog :: Console` (0.9s)
- `Camel :: Assembly` (0.7s)
- `Camel :: Endpoint DSL :: Support` (0.7s)
</details>
---
:gear: [View full build and test results](https://github.com/apache/camel/actions/runs/39196558511)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CAMEL-25742: camel-coap - cancel the observation when an observe consumer stops [camel]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #28069:
URL: https://github.com/apache/camel/pull/28069#issuecomment-6260619663
:star2: Thank you for your contribution to the Apache Camel project! :star2:
:robot: CI automation will test this PR automatically.
:camel: Apache Camel Committers, please review the following items:
* First-time contributors **require MANUAL approval** for the GitHub Actions to run
* You can use the command `/component-test (camel-)component-name1 (camel-)component-name2..` to request a test from the test bot although they are normally detected and executed by CI.
* You can label PRs using `skip-tests` and `test-dependents` to fine-tune the checks executed by this PR.
* Build and test logs are available in the summary page. **Only** [Apache Camel committers](https://camel.apache.org/community/team/#committers) have access to the summary.
:warning: Be careful when sharing logs. Review their contents before sharing them publicly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CAMEL-25742: camel-coap - cancel the observation when an observe consumer stops [camel]
Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus merged PR #28069:
URL: https://github.com/apache/camel/pull/28069
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org