BewareMyPower commented on code in PR #16969:
URL: https://github.com/apache/pulsar/pull/16969#discussion_r951071168


##########
pulsar-client-cpp/lib/MultiTopicsConsumerImpl.cc:
##########
@@ -363,13 +401,12 @@ void MultiTopicsConsumerImpl::closeAsync(ResultCallback 
callback) {
 
     auto self = shared_from_this();
     int numConsumers = 0;
-    consumers_.forEach(
-        [&numConsumers, &self, callback](const std::string& name, const 
ConsumerImplPtr& consumer) {
-            numConsumers++;
-            consumer->closeAsync([self, name, callback](Result result) {
-                self->handleSingleConsumerClose(result, name, callback);
-            });
+    for (const auto& item : consumers_.toPairVector()) {

Review Comment:
   > So there is a risk that the nullptr.
   
   At the very beginning, there is no chance that a `std::shared_ptr` becomes 
null unless the `reset()` method is explicitly called. You might mean the 
`std::shared_ptr` points to an invalid memory area.
   
   However, it's still impossible. First, the `forEachValue` method is 
protected by the internal lock, so the function argument accepts a non-null 
`consumer`. Second, even if the value was removed from the map when the 
callback of `closeAsync` method (`handleSingleConsumerClosed`) was executed, 
the object that the `ConsumerImplPtr` object points would still be valid. 
Third, the increment and decrement of the reference count of a 
`std::shared_ptr` is thread safe.
   
   The key point is that `ConsumerImplPtr` is a `std::shared_ptr`, not a 
trivial reference. The lifetime ends only when the reference count become 0. 
For example,
   
   ```c++
   #include <iostream>
   #include <memory>
   #include <vector>
   using namespace std;
   
   struct Object {
       Object() { cout << "ctor" << endl; }
       ~Object() { cout << "dtor" << endl; }
       void f() { cout << (void*)this << endl; }
   };
   
   int main(int argc, char* argv[]) {
       using ObjectPtr = std::shared_ptr<Object>;
       vector<ObjectPtr> objects;
       objects.emplace_back(std::make_shared<Object>());
       auto object = objects[0];
       cout << object.use_count() << endl;  // 2
       objects.clear();
       cout << object.use_count() << endl;  // 1
       object->f();                         // it works well because the 
reference count is still 1
       return 0;
   }
   ```



-- 
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]

Reply via email to