BewareMyPower commented on a change in pull request #10981:
URL: https://github.com/apache/pulsar/pull/10981#discussion_r655484022
##########
File path: pulsar-client-cpp/python/src/config.cc
##########
@@ -110,21 +112,29 @@ class LoggerWrapper: public Logger {
public:
- LoggerWrapper(const std::string &logger, PyObject* pyLogger) {
+ LoggerWrapper(const std::string &filename, PyObject* pyLogger) {
_pyLogger = pyLogger;
Py_XINCREF(_pyLogger);
+ fallbackLogger = (new ConsoleLoggerFactory())->getLogger(filename);
Review comment:
`new ConsoleLoggerFactory()` will cause memory leak. `getLogger` method
will also cause memory leak, see the implementation in
`lib/ConsoleLoggerFactoryImpl.h`:
```c++
Logger* getLogger(const std::string& fileName) { return new
SimpleLogger(std::cout, fileName, level_); }
```
The implementation is okay in C++ client because what `getLogger` returns
will be passed to the constructor of a thread local `std::unique_ptr` in
`lib/LogUtils.h`:
```c++
#define DECLARE_LOG_OBJECT()
\
static pulsar::Logger* logger() {
\
static thread_local std::unique_ptr<pulsar::Logger>
threadSpecificLogPtr; \
pulsar::Logger* ptr = threadSpecificLogPtr.get();
\
if (PULSAR_UNLIKELY(!ptr)) {
\
std::string logger = pulsar::LogUtils::getLoggerName(__FILE__);
\
threadSpecificLogPtr.reset(pulsar::LogUtils::getLoggerFactory()->getLogger(logger));
\
ptr = threadSpecificLogPtr.get();
\
}
\
return ptr;
\
}
```
And the what `new ConsoleLoggerFactory` returns will be passed to
`ClientConfiguration::setLogger`, which wraps the raw pointer into a
`std::shared_ptr`. See `lib/ClientConfiguration.cc`:
```c++
ClientConfiguration& ClientConfiguration::setLogger(LoggerFactory*
loggerFactory) {
impl_->loggerFactory.reset(loggerFactory);
return *this;
}
```
However, in Python wrapper, only the allocated `LoggerWrapper` will be
passed to the `setLogger`.
Therefore, you need to clean up the `fallbackLogger` in the destructor of
`LoggerWrapper`. As for the `ConsoleLoggerFactory`, it's better to use a
singleton.
--
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:
[email protected]
