clebertsuconic commented on code in PR #4407:
URL: https://github.com/apache/activemq-artemis/pull/4407#discussion_r1140316262
##########
artemis-server/src/test/java/org/apache/activemq/artemis/tests/util/ActiveMQTestBase.java:
##########
@@ -235,29 +222,40 @@ public MBeanServer getMBeanServer() {
// There is a verification about thread leakages. We only fail a single
thread when this happens
private static Set<Thread> alreadyFailedThread = new HashSet<>();
- private LinkedList<RunnableEx> runAfter;
+ private volatile List<RunnableEx> runAfter;
protected synchronized void runAfter(RunnableEx run) {
Assert.assertNotNull(run);
if (runAfter == null) {
- runAfter = new LinkedList();
+ runAfter = Collections.synchronizedList(new ArrayList<>());
}
runAfter.add(run);
}
@After
public void runAfter() {
- if (runAfter != null) {
- runAfter.forEach((r) -> {
+ List<RunnableEx> theRunafterList = runAfter;
+ if (theRunafterList != null) {
+ // I do this replacement as eventually a runAfter lambda could be
adding another runAfter.
+ // this shouldn't happen very often but I saw it happening once.
+ // it's better to be protected against
Review Comment:
I have a case where destroyDerby is being called as runAfter();
destroyDerby is calling the getURI, which will then turn runAfter back on.
On my case I had explicitly set runAFter() to be called even though JDBC was
not used.
So, it was an artificial case, but I thought It would be better to protect
it since I saw it happening.
all the calls to runAfter are from single threaded. I thought when I added
the volatile that it was multi-threaded.. but it's not the case.
--
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]
