Skip to content

Commit a200088

Browse files
zhaizhibozhaizhibo
andauthored
[fix][broker] Fix PulsarService.closeAsync where Condition.signalAll was called without holding a lock (apache#25777)
Co-authored-by: zhaizhibo <zhaizhibo@kuaishou.com>
1 parent 5a766b1 commit a200088

2 files changed

Lines changed: 35 additions & 2 deletions

File tree

pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -754,8 +754,13 @@ public CompletableFuture<Void> closeAsync(boolean waitForWebServiceToStop) {
754754
} else {
755755
log.warn().exception(t).log("Closed with errors");
756756
}
757-
state = State.Closed;
758-
isClosedCondition.signalAll();
757+
mutex.lock();
758+
try {
759+
state = State.Closed;
760+
isClosedCondition.signalAll();
761+
} finally {
762+
mutex.unlock();
763+
}
759764
return null;
760765
});
761766
return closeFuture;

pulsar-broker/src/test/java/org/apache/pulsar/broker/PulsarServiceCloseTest.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,12 @@
1818
*/
1919
package org.apache.pulsar.broker;
2020

21+
import static org.testng.Assert.fail;
2122
import static org.testng.AssertJUnit.assertFalse;
2223
import static org.testng.AssertJUnit.assertTrue;
24+
import java.util.concurrent.CompletableFuture;
2325
import java.util.concurrent.ScheduledFuture;
26+
import java.util.concurrent.TimeUnit;
2427
import lombok.CustomLog;
2528
import org.apache.commons.lang3.reflect.FieldUtils;
2629
import org.apache.pulsar.broker.auth.MockedPulsarServiceBaseTest;
@@ -73,4 +76,29 @@ public void closeInTimeTest() throws Exception {
7376
}
7477
}
7578

79+
@Test(timeOut = 60_000)
80+
public void testWaitUntilClosedConcurrentWithCloseAsync() throws Exception {
81+
// Start closeAsync() - it initiates close and returns a future
82+
CompletableFuture<Void> closeFuture = pulsar.closeAsync();
83+
84+
// Start waitUntilClosed() in a separate thread BEFORE close completes.
85+
// This thread will enter mutex.lock() -> await() and block there,
86+
// relying on signalAll() to be woken up when close finishes.
87+
CompletableFuture<Void> waitFuture = CompletableFuture.runAsync(() -> {
88+
try {
89+
pulsar.waitUntilClosed();
90+
} catch (InterruptedException e) {
91+
Thread.currentThread().interrupt();
92+
throw new RuntimeException(e);
93+
}
94+
});
95+
96+
try {
97+
closeFuture.get(30, TimeUnit.SECONDS);
98+
waitFuture.get(30, TimeUnit.SECONDS);
99+
} catch (Exception e) {
100+
fail("Should not throw exception");
101+
}
102+
log.info("waitUntilClosed() returned successfully while closeAsync() was in progress");
103+
}
76104
}

0 commit comments

Comments
 (0)