tabish121 commented on code in PR #5087:
URL: https://github.com/apache/activemq-artemis/pull/5087#discussion_r1676384995
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/management/impl/ManagementServiceImpl.java:
##########
@@ -719,10 +671,18 @@ public void activated() {
}
}
});
+
+ started = true;
}
@Override
public synchronized void stop() throws Exception {
+ if (!started) {
Review Comment:
Similar to above this doesn't really prevent concurrent calls to stop for
overlapping so this also needs to be idempotent. There is also the concern
that start / stop calls can run concurrently now which could leave this in an
odd state if not handled.
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/management/impl/ManagementServiceImpl.java:
##########
@@ -692,16 +644,16 @@ public SimpleString getManagementNotificationAddress() {
return managementNotificationAddress;
}
- // ActiveMQComponent implementation -----------------------------
-
@Override
- public void start() throws Exception {
+ public synchronized void start() throws Exception {
+ if (started) {
Review Comment:
This is a lot less protection than before for concurrent calls to start
since you aren't using an atomic operation to compare and set so you can get
two concurrent start calls competing in here. Would depend a lot on whether
the call is idempotent.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
