gnodet-bot commented on code in PR #28708:
URL: https://github.com/apache/camel/pull/28708#discussion_r4419324409
##########
components/camel-jgroups-raft/src/main/java/org/apache/camel/component/jgroups/raft/JGroupsRaftEndpoint.java:
##########
@@ -181,9 +213,20 @@ public void disconnect() {
((JGroupsRaftComponent)
getComponent()).disconnectSharedHandle(resolvedRaftHandle);
return;
}
- if (connectCount.decrementAndGet() == 0) {
- LOG.trace("Disconnecting JGroupsraft Channel {}",
getEndpointUri());
- resolvedRaftHandle.channel().disconnect();
+ connectLock.lock();
+ try {
+ if (--connectCount == 0) {
+ LOG.trace("Disconnecting JGroupsraft Channel {}",
getEndpointUri());
+ resolvedRaftHandle.channel().disconnect();
+ if (isStoppingOrStopped()) {
+ // the endpoint was stopped while this producer or
consumer still used the handle
+ closeOwnHandle();
+ }
Review Comment:
⚠️ **Resource leak:** `channel().disconnect()` and `closeOwnHandle()` share
the same try block. If `disconnect()` throws, `closeOwnHandle()` is skipped —
both the channel and log leak. Consider separating them:
```java
try {
resolvedRaftHandle.channel().disconnect();
} catch (Exception e) {
LOG.warn("Error disconnecting JGroupsraft channel of {}",
getEndpointUri(), e);
}
if (isStoppingOrStopped()) {
closeOwnHandle();
}
```
##########
components/camel-jgroups-raft/src/main/java/org/apache/camel/component/jgroups/raft/JGroupsRaftEndpoint.java:
##########
@@ -109,28 +112,51 @@ public void populateJGroupsRaftHeaders(Exchange exchange)
{
protected void doStart() throws Exception {
LOG.info("Resolving JGroupsraft handle {}", getEndpointUri());
super.doStart();
- resolvedRaftHandle = resolveHandle();
+ connectLock.lock();
+ try {
+ // the handle this endpoint created is kept when the endpoint was
stopped while producers or consumers
+ // still used it (such as when another route that consumes from
this endpoint was removed)
+ if (resolvedRaftHandle == null || !isOwnHandle() ||
resolvedRaftHandle.channel() == null
+ || resolvedRaftHandle.channel().isClosed()) {
+ resolvedRaftHandle = resolveHandle();
+ }
+ } finally {
+ connectLock.unlock();
+ }
}
@Override
protected void doStop() throws Exception {
// a handle this endpoint did not create (such as the raftHandle of
the component) is used by other endpoints,
// and is closed by the component
if (resolvedRaftHandle != null && isOwnHandle()) {
- LOG.info("Closing JGroupsraft Channel {}", getEndpointUri());
- if (resolvedRaftHandle.channel() != null) {
- resolvedRaftHandle.channel().close();
- LOG.info("Closed JGroupsraft Channel {}", getEndpointUri());
- }
- LOG.info("Closing Log {}", getEndpointUri());
- if (resolvedRaftHandle.log() != null) {
- resolvedRaftHandle.log().close();
- LOG.info("Closed Log Channel {}", getEndpointUri());
+ connectLock.lock();
+ try {
+ // the endpoint is stopped when a route that consumes from it
is removed, also when the producers and
+ // consumers of other routes still use its handle: then the
last of them that disconnects closes it
+ if (connectCount == 0) {
+ closeOwnHandle();
+ }
+ } finally {
+ connectLock.unlock();
}
}
super.doStop();
}
+ private void closeOwnHandle() throws Exception {
+ LOG.info("Closing JGroupsraft Channel {}", getEndpointUri());
+ if (resolvedRaftHandle.channel() != null) {
+ resolvedRaftHandle.channel().close();
+ LOG.info("Closed JGroupsraft Channel {}", getEndpointUri());
+ }
+ LOG.info("Closing Log {}", getEndpointUri());
+ if (resolvedRaftHandle.log() != null) {
+ resolvedRaftHandle.log().close();
Review Comment:
⚠️ **Resource leak:** If `channel().close()` (line 150) throws, this
`log().close()` call is never reached — the log leaks.
`JGroupsRaftComponent.doStop()` handles this with independent try/catch blocks
for each close. Consider the same pattern here:
```suggestion
private void closeOwnHandle() throws Exception {
LOG.info("Closing JGroupsraft Channel {}", getEndpointUri());
try {
if (resolvedRaftHandle.channel() != null) {
resolvedRaftHandle.channel().close();
LOG.info("Closed JGroupsraft Channel {}", getEndpointUri());
}
} catch (Exception e) {
LOG.warn("Error closing JGroupsraft channel of {}",
getEndpointUri(), e);
}
LOG.info("Closing Log {}", getEndpointUri());
if (resolvedRaftHandle.log() != null) {
resolvedRaftHandle.log().close();
LOG.info("Closed Log Channel {}", getEndpointUri());
}
}
```
--
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]
