Possibly fix deadlock happening due to unregister/register modules as switching between AST and TSO estimators.

I think this does not introduces any contention or new deadlocks. But that is hard to verify at the moment.

BUG=chromium:388191
R=stefan@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/17869004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@6582 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
andresp@webrtc.org 2014-07-02 13:23:19 +00:00
parent 0bb9fac98c
commit 1295dc6a23

View File

@ -32,12 +32,12 @@ static const uint32_t kTimeOffsetSwitchThreshold = 30;
class WrappingBitrateEstimator : public RemoteBitrateEstimator { class WrappingBitrateEstimator : public RemoteBitrateEstimator {
public: public:
WrappingBitrateEstimator(int engine_id, RemoteBitrateObserver* observer, WrappingBitrateEstimator(int engine_id,
Clock* clock, ProcessThread* process_thread, RemoteBitrateObserver* observer,
Clock* clock,
const Config& config) const Config& config)
: observer_(observer), : observer_(observer),
clock_(clock), clock_(clock),
process_thread_(process_thread),
crit_sect_(CriticalSectionWrapper::CreateCriticalSection()), crit_sect_(CriticalSectionWrapper::CreateCriticalSection()),
engine_id_(engine_id), engine_id_(engine_id),
min_bitrate_bps_(config.Get<RemoteBitrateEstimatorMinRate>().min_rate), min_bitrate_bps_(config.Get<RemoteBitrateEstimatorMinRate>().min_rate),
@ -48,13 +48,10 @@ class WrappingBitrateEstimator : public RemoteBitrateEstimator {
min_bitrate_bps_)), min_bitrate_bps_)),
using_absolute_send_time_(false), using_absolute_send_time_(false),
packets_since_absolute_send_time_(0) { packets_since_absolute_send_time_(0) {
assert(process_thread_ != NULL);
process_thread_->RegisterModule(rbe_.get());
}
virtual ~WrappingBitrateEstimator() {
process_thread_->DeRegisterModule(rbe_.get());
} }
virtual ~WrappingBitrateEstimator() {}
virtual void IncomingPacket(int64_t arrival_time_ms, virtual void IncomingPacket(int64_t arrival_time_ms,
int payload_size, int payload_size,
const RTPHeader& header) { const RTPHeader& header) {
@ -64,13 +61,13 @@ class WrappingBitrateEstimator : public RemoteBitrateEstimator {
} }
virtual int32_t Process() { virtual int32_t Process() {
assert(false && "Not supposed to register the WrappingBitrateEstimator!"); CriticalSectionScoped cs(crit_sect_.get());
return 0; return rbe_->Process();
} }
virtual int32_t TimeUntilNextProcess() { virtual int32_t TimeUntilNextProcess() {
assert(false && "Not supposed to register the WrappingBitrateEstimator!"); CriticalSectionScoped cs(crit_sect_.get());
return 0; return rbe_->TimeUntilNextProcess();
} }
virtual void OnRttUpdate(uint32_t rtt) { virtual void OnRttUpdate(uint32_t rtt) {
@ -133,7 +130,6 @@ class WrappingBitrateEstimator : public RemoteBitrateEstimator {
// Instantiate RBE for Time Offset or Absolute Send Time extensions. // Instantiate RBE for Time Offset or Absolute Send Time extensions.
void PickEstimator() EXCLUSIVE_LOCKS_REQUIRED(crit_sect_.get()) { void PickEstimator() EXCLUSIVE_LOCKS_REQUIRED(crit_sect_.get()) {
process_thread_->DeRegisterModule(rbe_.get());
if (using_absolute_send_time_) { if (using_absolute_send_time_) {
rbe_.reset(AbsoluteSendTimeRemoteBitrateEstimatorFactory().Create( rbe_.reset(AbsoluteSendTimeRemoteBitrateEstimatorFactory().Create(
observer_, clock_, rate_control_type_, min_bitrate_bps_)); observer_, clock_, rate_control_type_, min_bitrate_bps_));
@ -141,12 +137,10 @@ class WrappingBitrateEstimator : public RemoteBitrateEstimator {
rbe_.reset(RemoteBitrateEstimatorFactory().Create( rbe_.reset(RemoteBitrateEstimatorFactory().Create(
observer_, clock_, rate_control_type_, min_bitrate_bps_)); observer_, clock_, rate_control_type_, min_bitrate_bps_));
} }
process_thread_->RegisterModule(rbe_.get());
} }
RemoteBitrateObserver* observer_; RemoteBitrateObserver* observer_;
Clock* clock_; Clock* clock_;
ProcessThread* process_thread_;
scoped_ptr<CriticalSectionWrapper> crit_sect_; scoped_ptr<CriticalSectionWrapper> crit_sect_;
const int engine_id_; const int engine_id_;
const uint32_t min_bitrate_bps_; const uint32_t min_bitrate_bps_;
@ -176,14 +170,16 @@ ChannelGroup::ChannelGroup(int engine_id,
config_ = own_config_.get(); config_ = own_config_.get();
} }
assert(config_); // Must have a valid config pointer here. assert(config_); // Must have a valid config pointer here.
remote_bitrate_estimator_.reset( remote_bitrate_estimator_.reset(
new WrappingBitrateEstimator(engine_id, new WrappingBitrateEstimator(engine_id,
remb_.get(), remb_.get(),
Clock::GetRealTimeClock(), Clock::GetRealTimeClock(),
process_thread, *config_));
*config_)),
call_stats_->RegisterStatsObserver(remote_bitrate_estimator_.get()); call_stats_->RegisterStatsObserver(remote_bitrate_estimator_.get());
process_thread->RegisterModule(remote_bitrate_estimator_.get());
process_thread->RegisterModule(call_stats_.get()); process_thread->RegisterModule(call_stats_.get());
process_thread->RegisterModule(bitrate_controller_.get()); process_thread->RegisterModule(bitrate_controller_.get());
} }
@ -191,6 +187,7 @@ ChannelGroup::ChannelGroup(int engine_id,
ChannelGroup::~ChannelGroup() { ChannelGroup::~ChannelGroup() {
process_thread_->DeRegisterModule(bitrate_controller_.get()); process_thread_->DeRegisterModule(bitrate_controller_.get());
process_thread_->DeRegisterModule(call_stats_.get()); process_thread_->DeRegisterModule(call_stats_.get());
process_thread_->DeRegisterModule(remote_bitrate_estimator_.get());
call_stats_->DeregisterStatsObserver(remote_bitrate_estimator_.get()); call_stats_->DeregisterStatsObserver(remote_bitrate_estimator_.get());
assert(channels_.empty()); assert(channels_.empty());
assert(!remb_->InUse()); assert(!remb_->InUse());