shill: modem: unregister the cellular device when deleting a modem
Unregister the cellular device so that the chain of refences to the
device are all dropped when the modem disappears. The references are
from DeviceInfo, Manager, Modem and Service. Calling deregister
remove the device info reference, and will cause the manager to stop
the device, which will in turn clean up the service. Deleting the
modem will drop the last reference.
BUG=chromium-os:26796, chromium-os:26300
TEST=run shill, list-devices, stop cromo, list-devices, start cromo,
/opt/Qualcomm/bin/powercycle-all-gobis
Change-Id: Ia10932cc3c644bbf5accd69e02b85c75dc783b11
Reviewed-on: https://gerrit.chromium.org/gerrit/16546
Commit-Ready: Jason Glasgow <jglasgow@chromium.org>
Reviewed-by: Jason Glasgow <jglasgow@chromium.org>
Tested-by: Jason Glasgow <jglasgow@chromium.org>
diff --git a/cellular_capability.cc b/cellular_capability.cc
index 96b6a92..cda4eec 100644
--- a/cellular_capability.cc
+++ b/cellular_capability.cc
@@ -155,7 +155,9 @@
// has been converted to multi-step async.
void CellularCapability::DisableModem(AsyncCallHandler * /*call_handler*/) {
try {
- proxy_->Enable(false);
+ if (proxy_.get()) {
+ proxy_->Enable(false);
+ }
cellular()->OnModemDisabled();
} catch (const DBus::Error e) {
LOG(WARNING) << "Disable failed: " << e.what();
diff --git a/device_info.cc b/device_info.cc
index 1f8bd08..3419146 100644
--- a/device_info.cc
+++ b/device_info.cc
@@ -107,6 +107,24 @@
}
}
+void DeviceInfo::DeregisterDevice(const DeviceRefPtr &device) {
+ int interface_index = device->interface_index();
+
+ VLOG(2) << __func__ << "(" << device->link_name() << ", "
+ << interface_index << ")";
+ CHECK(device->TechnologyIs(Technology::kCellular));
+
+ // Release reference to the device
+ map<int, Info>::iterator iter = infos_.find(interface_index);
+ if (iter != infos_.end()) {
+ VLOG(2) << "Removing device from info for index: " << interface_index;
+ manager_->DeregisterDevice(device);
+ // Release the reference to the device, but maintain the mapping
+ // for the index. That will be cleaned up by an RTNL message.
+ iter->second.device = NULL;
+ }
+}
+
Technology::Identifier DeviceInfo::GetDeviceTechnology(
const string &iface_name) {
FilePath uevent_file(StringPrintf(kInterfaceUevent, iface_name.c_str()));
diff --git a/device_info.h b/device_info.h
index 571ff5b..f080fd0 100644
--- a/device_info.h
+++ b/device_info.h
@@ -55,6 +55,12 @@
// messages, and registers it with the manager.
void RegisterDevice(const DeviceRefPtr &device);
+ // Remove |device| from this DeviceInfo. This function should only
+ // be called for cellular devices because the lifetime of the
+ // cellular devices is controlled by the Modem object and its
+ // communication to modem manager, rather than by RTNL messages.
+ void DeregisterDevice(const DeviceRefPtr &device);
+
virtual DeviceRefPtr GetDevice(int interface_index) const;
virtual bool GetMACAddress(int interface_index, ByteString *address) const;
virtual bool GetFlags(int interface_index, unsigned int *flags) const;
diff --git a/modem.cc b/modem.cc
index c61ba76..0d4711b 100644
--- a/modem.cc
+++ b/modem.cc
@@ -43,7 +43,11 @@
LOG(INFO) << "Modem created: " << owner << " at " << path;
}
-Modem::~Modem() {}
+Modem::~Modem() {
+ if (device_.get()) {
+ manager_->device_info()->DeregisterDevice(device_);
+ }
+}
void Modem::Init() {
VLOG(2) << __func__;
diff --git a/modem_unittest.cc b/modem_unittest.cc
index a053f78..f658c21 100644
--- a/modem_unittest.cc
+++ b/modem_unittest.cc
@@ -52,15 +52,16 @@
public:
ModemTest()
: manager_(&control_interface_, &dispatcher_, &metrics_, &glib_),
+ info_(&control_interface_, &dispatcher_, &metrics_, &manager_),
proxy_(new MockDBusPropertiesProxy()),
proxy_factory_(this),
- modem_(kOwner,
- kPath,
- &control_interface_,
- &dispatcher_,
- &metrics_,
- &manager_,
- NULL) {}
+ modem_(new Modem(kOwner,
+ kPath,
+ &control_interface_,
+ &dispatcher_,
+ &metrics_,
+ &manager_,
+ NULL)) {}
virtual void SetUp();
virtual void TearDown();
@@ -90,7 +91,7 @@
CellularCapabilityGSM *GetCapabilityGSM() {
return dynamic_cast<CellularCapabilityGSM *>(
- modem_.device_->capability_.get());
+ modem_->device_->capability_.get());
}
MockGLib glib_;
@@ -98,9 +99,10 @@
EventDispatcher dispatcher_;
MockMetrics metrics_;
MockManager manager_;
+ MockDeviceInfo info_;
scoped_ptr<MockDBusPropertiesProxy> proxy_;
TestProxyFactory proxy_factory_;
- Modem modem_;
+ scoped_ptr<Modem> modem_;
StrictMock<MockSockets> sockets_;
};
@@ -108,26 +110,27 @@
const char ModemTest::kPath[] = "/org/chromium/ModemManager/Gobi/0";
void ModemTest::SetUp() {
- EXPECT_EQ(kOwner, modem_.owner_);
- EXPECT_EQ(kPath, modem_.path_);
- modem_.proxy_factory_ = &proxy_factory_;
- SetSockets(&sockets_);
+ EXPECT_EQ(kOwner, modem_->owner_);
+ EXPECT_EQ(kPath, modem_->path_);
+ modem_->proxy_factory_ = &proxy_factory_;
}
void ModemTest::TearDown() {
- modem_.proxy_factory_ = NULL;
+ modem_.reset();
SetSockets(NULL);
}
TEST_F(ModemTest, Init) {
DBusPropertiesMap props;
+
+ SetSockets(&sockets_);
props[Modem::kPropertyIPMethod].writer().append_uint32(
MM_MODEM_IP_METHOD_DHCP);
props[Modem::kPropertyLinkName].writer().append_string("usb1");
EXPECT_CALL(*proxy_, GetAll(MM_MODEM_INTERFACE)).WillOnce(Return(props));
- EXPECT_TRUE(modem_.task_factory_.empty());
- modem_.Init();
- EXPECT_FALSE(modem_.task_factory_.empty());
+ EXPECT_TRUE(modem_->task_factory_.empty());
+ modem_->Init();
+ EXPECT_FALSE(modem_->task_factory_.empty());
EXPECT_CALL(sockets_, Socket(PF_INET, SOCK_DGRAM, 0)).WillOnce(Return(-1));
dispatcher_.DispatchPendingEvents();
@@ -136,23 +139,24 @@
TEST_F(ModemTest, CreateDeviceFromProperties) {
DBusPropertiesMap props;
- modem_.CreateDeviceFromProperties(props);
- EXPECT_FALSE(modem_.device_.get());
+ modem_->CreateDeviceFromProperties(props);
+ EXPECT_FALSE(modem_->device_.get());
props[Modem::kPropertyIPMethod].writer().append_uint32(
MM_MODEM_IP_METHOD_PPP);
- modem_.CreateDeviceFromProperties(props);
- EXPECT_FALSE(modem_.device_.get());
+ modem_->CreateDeviceFromProperties(props);
+ EXPECT_FALSE(modem_->device_.get());
props.erase(Modem::kPropertyIPMethod);
props[Modem::kPropertyIPMethod].writer().append_uint32(
MM_MODEM_IP_METHOD_DHCP);
- modem_.CreateDeviceFromProperties(props);
- EXPECT_FALSE(modem_.device_.get());
+ modem_->CreateDeviceFromProperties(props);
+ EXPECT_FALSE(modem_->device_.get());
static const char kLinkName[] = "usb0";
static const unsigned char kAddress[] = {0x00, 0x01, 0x02, 0x03, 0x04, 0x05};
const int kTestSocket = 10;
+ const int kTestLinkSocket = 11;
props[Modem::kPropertyLinkName].writer().append_string(kLinkName);
EXPECT_CALL(sockets_, Socket(PF_INET, SOCK_DGRAM, 0))
.Times(2)
@@ -162,17 +166,24 @@
EXPECT_CALL(sockets_, Close(kTestSocket))
.WillRepeatedly(Return(0));
+ EXPECT_CALL(sockets_, Socket(PF_NETLINK, _, _))
+ .WillRepeatedly(Return(kTestLinkSocket));
+ EXPECT_CALL(sockets_, Bind(kTestLinkSocket, _, _))
+ .WillRepeatedly(Return(0));
+ EXPECT_CALL(sockets_, Send(kTestLinkSocket, _, _, _))
+ .WillRepeatedly(Return(0));
+ RTNLHandler::GetInstance()->Start(&dispatcher_, &sockets_);
+
ByteString expected_address(kAddress, arraysize(kAddress));
- MockDeviceInfo info_(&control_interface_, &dispatcher_, &metrics_, &manager_);
EXPECT_CALL(info_, GetMACAddress(kTestInterfaceIndex, _))
.WillOnce(DoAll(SetArgumentPointee<1>(expected_address), Return(true)))
.WillOnce(DoAll(SetArgumentPointee<1>(expected_address), Return(true)));
EXPECT_CALL(info_, GetDevice(kTestInterfaceIndex))
- .WillRepeatedly(Return(modem_.device_));
+ .WillRepeatedly(Return(modem_->device_));
EXPECT_CALL(manager_, device_info()).WillRepeatedly(Return(&info_));
- modem_.CreateDeviceFromProperties(props);
- EXPECT_FALSE(modem_.device_.get());
+ modem_->CreateDeviceFromProperties(props);
+ EXPECT_FALSE(modem_->device_.get());
props[Modem::kPropertyType].writer().append_uint32(MM_MODEM_TYPE_GSM);
props[Modem::kPropertyState].writer().append_uint32(
@@ -185,11 +196,11 @@
kLockType);
props[CellularCapabilityGSM::kPropertyUnlockRetries].writer().append_uint32(
kRetries);
- modem_.CreateDeviceFromProperties(props);
- ASSERT_TRUE(modem_.device_.get());
- EXPECT_EQ(kLinkName, modem_.device_->link_name());
- EXPECT_EQ(kTestInterfaceIndex, modem_.device_->interface_index());
- EXPECT_EQ(Cellular::kModemStateDisabled, modem_.device_->modem_state());
+ modem_->CreateDeviceFromProperties(props);
+ ASSERT_TRUE(modem_->device_.get());
+ EXPECT_EQ(kLinkName, modem_->device_->link_name());
+ EXPECT_EQ(kTestInterfaceIndex, modem_->device_->interface_index());
+ EXPECT_EQ(Cellular::kModemStateDisabled, modem_->device_->modem_state());
EXPECT_TRUE(GetCapabilityGSM()->sim_lock_status_.enabled);
EXPECT_EQ(kLockType, GetCapabilityGSM()->sim_lock_status_.lock_type);
EXPECT_EQ(kRetries, GetCapabilityGSM()->sim_lock_status_.retries_left);
@@ -197,7 +208,7 @@
vector<DeviceRefPtr> devices;
manager_.FilterByTechnology(Technology::kCellular, &devices);
EXPECT_EQ(1, devices.size());
- EXPECT_TRUE(devices[0].get() == modem_.device_.get());
+ EXPECT_TRUE(devices[0].get() == modem_->device_.get());
}
} // namespace shill