You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "BewareMyPower (via GitHub)" <gi...@apache.org> on 2023/03/20 05:23:17 UTC
[GitHub] [pulsar-client-cpp] BewareMyPower opened a new pull request, #223: Make stats timers thread safe to use
BewareMyPower opened a new pull request, #223:
URL: https://github.com/apache/pulsar-client-cpp/pull/223
### Motivation
The timers' callbacks in `ProducerStatsImpl` and `ConsumerStatsImpl` are not thread safe because they both capture the `this` pointer, while when the callback is called in the event loop, `this` might point to an expired instance.
### Modifications
- Capture the weak pointer instead of `this` in these callbacks. Since we cannot call `shared_from_this()` in the constructor, a `start()` method is added.
- Remove the useless `executor_` field and unnecessary null check for `timer_`.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-cpp] BewareMyPower commented on pull request #223: Make stats timers thread safe to use
Posted by "BewareMyPower (via GitHub)" <gi...@apache.org>.
BewareMyPower commented on PR #223:
URL: https://github.com/apache/pulsar-client-cpp/pull/223#issuecomment-1475705463
It seems there are some tests failing. I'll fix them ASAP.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-cpp] BewareMyPower merged pull request #223: Make stats timers thread safe to use
Posted by "BewareMyPower (via GitHub)" <gi...@apache.org>.
BewareMyPower merged PR #223:
URL: https://github.com/apache/pulsar-client-cpp/pull/223
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-cpp] BewareMyPower commented on a diff in pull request #223: Make stats timers thread safe to use
Posted by "BewareMyPower (via GitHub)" <gi...@apache.org>.
BewareMyPower commented on code in PR #223:
URL: https://github.com/apache/pulsar-client-cpp/pull/223#discussion_r1147192180
##########
lib/stats/ConsumerStatsImpl.cc:
##########
@@ -57,23 +53,20 @@ void ConsumerStatsImpl::flushAndReset(const boost::system::error_code& ec) {
}
Lock lock(mutex_);
- ConsumerStatsImpl tmp = *this;
+ std::ostringstream oss;
Review Comment:
Why? We need to log the current instance. Before this PR, it was logged by copying the current instance, which is heavy and might cause unexpected destruction of the shared pointer.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-cpp] addisonj commented on a diff in pull request #223: Make stats timers thread safe to use
Posted by "addisonj (via GitHub)" <gi...@apache.org>.
addisonj commented on code in PR #223:
URL: https://github.com/apache/pulsar-client-cpp/pull/223#discussion_r1142436544
##########
lib/stats/ConsumerStatsImpl.cc:
##########
@@ -57,23 +53,20 @@ void ConsumerStatsImpl::flushAndReset(const boost::system::error_code& ec) {
}
Lock lock(mutex_);
- ConsumerStatsImpl tmp = *this;
+ std::ostringstream oss;
Review Comment:
This should be removed
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org