Skip to content

Commit 562b933

Browse files
Add Missing Synchronization in Ack Buffer Access (#1038)
Fixes #1029 There was one piece of missing synchronization to the Acknowledgement Buffer that led to a racing condition where acknowledgements could be lost. This commit adds the missing synchronization to fix the issue.
1 parent 49d7ef4 commit 562b933

2 files changed

Lines changed: 37 additions & 14 deletions

File tree

spring-cloud-aws-sqs/src/main/java/io/awspring/cloud/sqs/listener/acknowledgement/BatchingAcknowledgementProcessor.java

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -191,8 +191,7 @@ public void run() {
191191
try {
192192
Message<T> polledMessage = this.acks.poll(1, TimeUnit.SECONDS);
193193
if (polledMessage != null) {
194-
this.acksBuffer.computeIfAbsent(this.messageGroupingFunction.apply(polledMessage),
195-
newGroup -> new LinkedBlockingQueue<>()).add(polledMessage);
194+
addMessageToBuffer(polledMessage);
196195
this.thresholdAcknowledgementExecution.checkAndExecute();
197196
}
198197
}
@@ -203,6 +202,17 @@ public void run() {
203202
logger.debug("Acknowledgement processor thread stopped");
204203
}
205204

205+
private void addMessageToBuffer(Message<T> polledMessage) {
206+
this.context.lock();
207+
try {
208+
this.acksBuffer.computeIfAbsent(this.messageGroupingFunction.apply(polledMessage),
209+
newGroup -> new LinkedBlockingQueue<>()).add(polledMessage);
210+
}
211+
finally {
212+
this.context.unlock();
213+
}
214+
}
215+
206216
public void waitAcknowledgementsToFinish() {
207217
try {
208218
CompletableFuture.allOf(this.context.runningAcks.toArray(new CompletableFuture[] {}))
@@ -330,16 +340,11 @@ private boolean isRunning() {
330340
}
331341

332342
private void purgeEmptyBuffers() {
333-
lock();
334-
try {
335-
List<String> emptyAcks = this.acksBuffer.entrySet().stream().filter(entry -> entry.getValue().isEmpty())
336-
.map(Map.Entry::getKey).collect(Collectors.toList());
337-
logger.trace("Removing groups {} from buffer in {}", emptyAcks, this.id);
338-
emptyAcks.forEach(this.acksBuffer::remove);
339-
}
340-
finally {
341-
unlock();
342-
}
343+
verifyLock();
344+
List<String> emptyAcks = this.acksBuffer.entrySet().stream().filter(entry -> entry.getValue().isEmpty())
345+
.map(Map.Entry::getKey).collect(Collectors.toList());
346+
logger.trace("Removing groups {} from buffer in {}", emptyAcks, this.id);
347+
emptyAcks.forEach(this.acksBuffer::remove);
343348
}
344349

345350
private void lock() {

spring-cloud-aws-sqs/src/test/java/io/awspring/cloud/sqs/integration/BaseSqsIntegrationTest.java

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,9 @@ abstract class BaseSqsIntegrationTest {
4444

4545
protected static final boolean useLocalStackClient = true;
4646

47-
protected static final boolean purgeQueues = true;
47+
protected static boolean purgeQueues = true;
48+
49+
protected static boolean waitForPurge = true;
4850

4951
private static final String LOCAL_STACK_VERSION = "localstack/localstack:2.3.2";
5052

@@ -93,7 +95,14 @@ protected static CompletableFuture<?> createQueue(SqsAsyncClient client, String
9395
if (purgeQueues) {
9496
String queueUrl = v.queueUrl();
9597
logger.debug("Purging queue {}", queueName);
96-
return client.purgeQueue(req -> req.queueUrl(queueUrl).build());
98+
return client.purgeQueue(req -> req.queueUrl(queueUrl).build())
99+
.thenRun(() -> {
100+
if (waitForPurge) {
101+
logger.info("Waiting 30000 seconds to start sending.");
102+
sleep(30000);
103+
logger.info("Done waiting.");
104+
}
105+
});
97106
}
98107
else {
99108
logger.debug("Skipping purge for queue {}", queueName);
@@ -108,6 +117,15 @@ protected static CompletableFuture<?> createQueue(SqsAsyncClient client, String
108117
});
109118
}
110119

120+
private static void sleep(int time) {
121+
try {
122+
Thread.sleep(time);
123+
} catch (InterruptedException e) {
124+
Thread.currentThread().interrupt();
125+
throw new RuntimeException("Interrupted while sleeping");
126+
}
127+
}
128+
111129
private static CreateQueueRequest getCreateQueueRequest(String queueName,
112130
Map<QueueAttributeName, String> attributes, CreateQueueRequest.Builder builder) {
113131
if (!attributes.isEmpty()) {

0 commit comments

Comments
 (0)