shill: Change cellular callback bindings to weak pointers.
Change to using weak pointers in DBus callback bindings so that we do
not hold a reference count out on the object while calling into modem
manager. This becomes a problem if an async DBus call is in progress
and we release the proxy. The callback does not get executed which
leaves a dangling reference count and keeps the object alive. This stale
DBus object prevents us from properly registering the new DBus object
since the two paths are the same.
BUG=chromium-os:31636
TEST=Unit tests, network_3GSuspendResume
Change-Id: I252f9e28978b5426509bfafdd66adbfd8b289936
Reviewed-on: https://gerrit.chromium.org/gerrit/29667
Reviewed-by: mukesh agrawal <quiche@chromium.org>
Reviewed-by: Darin Petkov <petkov@chromium.org>
Tested-by: Thieu Le <thieule@chromium.org>
Commit-Ready: Thieu Le <thieule@chromium.org>
diff --git a/cellular.cc b/cellular.cc
index a69eaa2..f5c7ce0 100644
--- a/cellular.cc
+++ b/cellular.cc
@@ -108,6 +108,7 @@
address,
interface_index,
Technology::kCellular),
+ weak_ptr_factory_(this),
state_(kStateDisabled),
modem_state_(kModemStateUnknown),
dbus_owner_(owner),
@@ -204,15 +205,19 @@
if (state_ != kStateDisabled) {
return;
}
- capability_->StartModem(error,
- Bind(&Cellular::StartModemCallback, this, callback));
+ ResultCallback cb = Bind(&Cellular::StartModemCallback,
+ weak_ptr_factory_.GetWeakPtr(),
+ callback);
+ capability_->StartModem(error, cb);
}
void Cellular::Stop(Error *error,
const EnabledStateChangedCallback &callback) {
SLOG(Cellular, 2) << __func__ << ": " << GetStateString(state_);
- capability_->StopModem(error,
- Bind(&Cellular::StopModemCallback, this, callback));
+ ResultCallback cb = Bind(&Cellular::StopModemCallback,
+ weak_ptr_factory_.GetWeakPtr(),
+ callback);
+ capability_->StopModem(error, cb);
}
bool Cellular::IsUnderlyingDeviceEnabled() const {
@@ -404,9 +409,8 @@
DBusPropertiesMap properties;
capability_->SetupConnectProperties(&properties);
- // TODO(njw): Should a weak pointer be used here instead?
- // Would require something like a WeakPtrFactory on the class.
- ResultCallback cb = Bind(&Cellular::OnConnectReply, this);
+ ResultCallback cb = Bind(&Cellular::OnConnectReply,
+ weak_ptr_factory_.GetWeakPtr());
OnConnecting();
capability_->Connect(properties, error, cb);
}
@@ -455,8 +459,8 @@
error, Error::kNotConnected, "Not connected; request ignored.");
return;
}
- // TODO(njw): See Connect() about possible weak ptr for 'this'.
- ResultCallback cb = Bind(&Cellular::OnDisconnectReply, this);
+ ResultCallback cb = Bind(&Cellular::OnDisconnectReply,
+ weak_ptr_factory_.GetWeakPtr());
capability_->Disconnect(error, cb);
}