shill: fix crash in IPConfig::UpdateProperties
If UpdateProperties reports failure to |update_callback_|, the
device owning the IPConfig object may drop its reference to
the IPConfig. We'll then crash, when we try to EmitChanges.
Avoid the crash by having UpdateProperties hold a reference
to the IPConfig object for the duration of the method call.
BUG=chromium:237576
TEST=new unit test
Change-Id: Ia3ce1a4729429ec75d4aab6b4c84b67f573d9187
Reviewed-on: https://gerrit.chromium.org/gerrit/50509
Reviewed-by: Paul Stewart <pstew@chromium.org>
Commit-Queue: mukesh agrawal <quiche@chromium.org>
Tested-by: mukesh agrawal <quiche@chromium.org>
diff --git a/ipconfig.cc b/ipconfig.cc
index 4d8fede..ce8c9c1 100644
--- a/ipconfig.cc
+++ b/ipconfig.cc
@@ -122,6 +122,11 @@
}
void IPConfig::UpdateProperties(const Properties &properties, bool success) {
+ // Take a reference of this instance to make sure we don't get destroyed in
+ // the middle of this call. (The |update_callback_| may cause a reference
+ // to be dropped. See, e.g., EthernetService::Disconnect and
+ // Ethernet::DropConnection.)
+ IPConfigRefPtr me = this;
properties_ = properties;
if (!update_callback_.is_null()) {
update_callback_.Run(this, success);
diff --git a/ipconfig_unittest.cc b/ipconfig_unittest.cc
index 3308ada..91b18bb 100644
--- a/ipconfig_unittest.cc
+++ b/ipconfig_unittest.cc
@@ -34,6 +34,9 @@
class IPConfigTest : public Test {
public:
IPConfigTest() : ipconfig_(new IPConfig(&control_, kDeviceName)) {}
+ void DropRef(const IPConfigRefPtr &/*ipconfig*/, bool /*config_success*/) {
+ ipconfig_ = NULL;
+ }
protected:
IPConfigMockAdaptor *GetAdaptor() {
@@ -146,6 +149,14 @@
}
}
+TEST_F(IPConfigTest, UpdatePropertiesWithDropRef) {
+ // The UpdateCallback should be able to drop a reference to the
+ // IPConfig object without crashing.
+ ipconfig_->RegisterUpdateCallback(
+ Bind(&IPConfigTest::DropRef, Unretained(this)));
+ UpdateProperties(IPConfig::Properties(), true);
+}
+
TEST_F(IPConfigTest, PropertyChanges) {
IPConfigMockAdaptor *adaptor = GetAdaptor();