Fix null MessageId may be passed to its compareTo() method

[BewareMyPower]

Fixes #11604

Motivation

There's a chance that null MessageId might be passed to its compareTo() method. Even if #10586 added the null check, it's still not thread safe because lastCumulativeAck might be modified after the null check between line 118 and line 121 in following code snippet:

pulsar/pulsar-client/src/main/java/org/apache/pulsar/client/impl/PersistentAcknowledgmentsGroupingTracker.java

Lines 118 to 121 in 1e9f0cd

	if (lastCumulativeAck.messageId == null) {
	return false;
	}
	if (messageId.compareTo(lastCumulativeAck.messageId) <= 0) {

Modifications

Add a local variable messageIdOfLastAck to cache messageId field of lastCumulative so that the MessageId won't change during the null check and compareTo() method.

Verifying this change

• Make sure that the change passes the CI checks.

This change is a trivial rework / code cleanup without any test coverage.

[BewareMyPower]

@leizhiyuan Please also take a look.

[BewareMyPower]

@leizhiyuan

if you changed lastCumulativeAck.messageId to null, messageIdOfLastAck also will be null.

I think not. They both reference to the same object, but if you set one of them to null, the other still references to the original object.

[leizhiyuan]

ok👍

[congbobo184]

LGTM!