Skip to content

Commit 4f3fcd8

Browse files
fix(pubsub): count a published message while its batch lock is held
The failure callback decrements messagesWaiter for the messages it cancels out of a MessagesBatch, but publish() incremented after releasing messagesBatchLock. A message visible in the batch and not yet counted would therefore be decremented for without ever having been counted, taking pendingCount below zero and letting waitComplete() return early. Incrementing while the lock is still held makes "in a MessagesBatch" and "counted" one state. Lock ordering is messagesBatchLock -> Waiter monitor here and nowhere the reverse, and incrementPendingCount never blocks. The paused-key path still returns before the increment, as before.
1 parent 330af86 commit 4f3fcd8

1 file changed

Lines changed: 6 additions & 2 deletions

File tree

  • java-pubsub/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1

java-pubsub/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/Publisher.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,12 @@ public ApiFuture<String> publish(PubsubMessage message) {
322322
}
323323

324324
batchesToSend = messagesBatch.add(outstandingPublish);
325+
// Counted while messagesBatchLock is still held, so that "in a MessagesBatch" and "counted"
326+
// are one state. The failure callback decrements for the messages it cancels out of a
327+
// MessagesBatch, so one that is visible there but not yet counted would take pendingCount
328+
// below zero. Lock ordering is messagesBatchLock -> Waiter monitor here and nowhere the
329+
// reverse, and incrementPendingCount never blocks.
330+
messagesWaiter.incrementPendingCount(1);
325331
if (!batchesToSend.isEmpty() && messagesBatch.isEmpty()) {
326332
messagesBatches.remove(orderingKey);
327333
}
@@ -340,8 +346,6 @@ public ApiFuture<String> publish(PubsubMessage message) {
340346
messagesBatchLock.unlock();
341347
}
342348

343-
messagesWaiter.incrementPendingCount(1);
344-
345349
// For messages without ordering keys, it is okay to send batches without holding
346350
// messagesBatchLock.
347351
if (!batchesToSend.isEmpty() && orderingKey.isEmpty()) {

0 commit comments

Comments
 (0)