Skip to content

Commit

Permalink
Fix PublishCallCh.afterAckCallback race condition
Browse files Browse the repository at this point in the history
Related to https://stackoverflow.com/questions/66071476/impossible-nullpointerexception-springframework-rabbitmq-failed-to-invoke

When we have several pending confirms, we handle them concurrently
in the `doHandleConfirm()`.
The `afterAckCallback` can be set `null` in one thread and
another will fail with NPE after acquiring a monitor from the
`getPendingConfirmsCount()`.

* Extract a local variable for the `callback` inside an `if` block
* Initialize it with `this.afterAckCallback` only if
`getPendingConfirmsCount() == 0` is true,
and set `this.afterAckCallback` to null inside `synchronized (this)`
block
* Call `callback.accept(this)` when local variable is not `null`
and outside of `synchronized` for better performance on end-user callback

**Cherry-pick to 2.2.x**
  • Loading branch information
artembilan authored and garyrussell committed Feb 8, 2021
1 parent ba4ef38 commit faa7dce
Showing 1 changed file with 12 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1026,9 +1026,18 @@ private void doHandleConfirm(boolean ack, Listener listener, PendingConfirm pend
}
finally {
try {
if (this.afterAckCallback != null && getPendingConfirmsCount() == 0) {
this.afterAckCallback.accept(this);
this.afterAckCallback = null;
if (this.afterAckCallback != null) {
java.util.function.Consumer<Channel> callback = null;
synchronized (this) {
if (getPendingConfirmsCount() == 0) {
callback = this.afterAckCallback;
this.afterAckCallback = null;
}
}
if (callback != null) {
callback.accept(this);
}

}
}
catch (Exception e) {
Expand Down

0 comments on commit faa7dce

Please sign in to comment.