BewareMyPower commented on code in PR #15726:
URL: https://github.com/apache/pulsar/pull/15726#discussion_r880035306
##########
pulsar-client-cpp/lib/ConsumerImpl.cc:
##########
@@ -722,11 +722,14 @@ Result
ConsumerImpl::fetchSingleMessageFromBroker(Message& msg) {
sendFlowPermitsToBroker(currentCnx, 1);
while (true) {
- incomingMessages_.pop(msg);
+ if (!incomingMessages_.pop(msg)) {
+ return ResultInterrupted;
+ }
+
{
// Lock needed to prevent race between connectionOpened and the
check "msg.impl_->cnx_ ==
// currentCnx.get())"
- Lock localLock(mutex_);
+ Lock localLock1(mutex_);
Review Comment:
This change is not needed. The name of the local variable could shadow the
variable outside the block.
##########
pulsar-client-cpp/lib/ProducerImpl.cc:
##########
@@ -526,9 +526,13 @@ int ProducerImpl::getNumOfChunks(uint32_t size, uint32_t
maxMessageSize) {
Result ProducerImpl::canEnqueueRequest(uint32_t payloadSize) {
if (conf_.getBlockIfQueueFull()) {
if (semaphore_) {
- semaphore_->acquire();
+ if (!semaphore_->acquire()) {
+ return ResultInterrupted;
+ }
Review Comment:
We can merge two `if` clauses to
```c++
if (semaphore_ && !semaphore_->acquire()) {
return ResultInterrupted;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
