[GitHub] [pulsar] BewareMyPower commented on pull request #10981: [python-client] Fixed crash when using Python logger

[GitBox <gi...@apache.org>]

BewareMyPower commented on pull request #10981:
URL: https://github.com/apache/pulsar/pull/10981#issuecomment-866480642


   There're a chance of memory leak theoretically but **it might never happen**.
   
   ```c++
   // copy constructor
   LoggerWrapper(const LoggerWrapper& other) {
       /* ... */
       fallbackLogger = other.fallbackLogger;
   }
   // copy assignment operator
   LoggerWrapper& operator=(const LoggerWrapper& other) { /* ... */ }
   ```
   
   If you managed a raw pointer `fallbackLogger`, these methods may cause that two `LoggerWrapper` objects share the same `fallbackLogger` but no reference count is recorded. So the pointer (`fallbackLogger`) could be deleted twice for each destructor of `LoggerWrapper`.
   
   There're two ways to fix it. One way is just to remove these two methods. Because currently no copy of `Logger` happens in Pulsar C++ library's implementation. These two methods could never be called so I said the memory leak might never happen before.
   
   The other way is use a `std::shared_ptr<Logger>` instead the raw pointer `Logger`. `std::shared_ptr` maintains a reference count for the pointer and the destructor would be called only once when the reference count became zero.
   
   Just change 
   
   ```c++
       Logger* fallbackLogger;
   ```
   
   to
   
   ```c++
       std::shared_ptr<Logger> fallbackLogger;
   ```
   
   And change
   
   ```c++
   fallbackLogger = factory->getLogger(filename);
   ```
   
   to
   
   ```c++
   fallbackLogger.reset(factory->getLogger(filename));
   ```
   
   Finally remove `delete fallbackLogger` in the destructor.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org