oscerd opened a new pull request, #26533:
URL: https://github.com/apache/camel/pull/26533
Found by a source audit of `components/camel-vertx`.
### The problem
`VertxWebsocketHostKey` is `{host, port}` only, so every consumer on the
same port shares one
`VertxWebsocketHost` (`VertxWebsocketComponent:167`, `computeIfAbsent`). But
the host was removed from the
registry by whichever consumer stopped first:
```java
VertxWebsocketHost vertxWebsocketHost = vertxHostRegistry.remove(hostKey);
if (vertxWebsocketHost != null) {
vertxWebsocketHost.disconnect(configuration.getWebsocketURI().getPath());
}
```
With consumers on `/test/a` and `/test/b` sharing a port:
* stopping `a` removes the shared host from the registry and unregisters
route `a`. The host correctly keeps
its server up, because `b` is still in its own `routeRegistry`;
* stopping `b` then finds nothing — `remove` returns null, the guard skips,
and **route `b` is never removed
and its server never stops**, outliving every Camel route;
* starting a consumer on that host and port again builds a *second* host
with a new Router and calls
`start()` on a port the first server still holds.
`doStop` closes Vert.x only when Camel created it, so with an
application-supplied `Vertx` or `Router` —
Spring Boot, Quarkus — nothing tears the server down at all.
Multiple consumers per port is supported and tested behaviour
(`VertxWebsocketMultiConsumerTest`), so this
is reachable in an ordinary configuration.
### The change
The host is looked up rather than removed, and leaves the registry only once
it serves no consumer — which
is the same moment it stops its own server. `isServingConsumers()` is new
and additive; `disconnect` keeps
its signature, since `VertxWebsocketHost` is public.
Two smaller problems in the same lifecycle go with it:
* `routeRegistry` becomes a `ConcurrentHashMap`. It is mutated by
`connect`/`disconnect`, i.e. as the route
controller starts and stops consumers, which it can do concurrently; the
field immediately below it was
already a `CopyOnWriteArrayList` carrying a `// thread-safe` comment, so
the difference read as an
oversight rather than a decision.
* `disconnect` no longer calls `route.remove()` on a null route when handed
a path it does not serve.
### Tests
`VertxWebsocketMultiConsumerLifecycleTest` (new, 3 cases) covers two
consumers sharing a port: the host
survives the first stop and the surviving consumer still receives; the host
is gone after the last stop; and
stop/start/stop of one consumer leaves the registry intact.
**Checked that the tests actually catch this**, rather than trusting green:
with the component change
reverted, 2 of the 3 fail (`theHostOutlivesTheFirstConsumerToStop`,
`stoppingAConsumerTwiceIsHarmless`). The
third passes either way — it is the degenerate case, kept because it pins
the intended end state.
`mvn clean install -DskipITs` on `components/camel-vertx` is green — 88
tests. Full reactor
`mvn clean install -DskipTests -DskipITs -Dquickly` green.
`org.assertj:assertj-core` was not on this module's test classpath and is
added.
---
_Claude Code on behalf of oscerd_
🤖 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
