+- OpenHome Forum (http://forum.openhome.org)
+-- Forum: OpenHome (/forumdisplay.php?fid=1)
+--- Forum: Net (/forumdisplay.php?fid=5)
+--- Thread: Device stack crashes when network adapter disabled (/showthread.php?tid=492)
Device stack crashes when network adapter disabled - simoncn - 30-08-2011 09:57 PM
If the device stack is started (idle) and the network adapter that it's using is disabled, the stack crashes. It should be able to survive an adapter disable/enable sequence.
I found the following three problems and fixed them. With these changes, the adapter can be disabled and re-enabled with the stack still continuing to work afterwards.
1) There's a recursive mutex lock assertion failure when deleting the disabled adapter. The code in NetworkAdapterList::HandleInterfaceListChanged locks the mutex iListLock at the start and is still holding this lock when it executes the following code:
Code:
if (subnetsChanged) {
RunCallbacks(iListenersSubnet);
}I fixed this by moving the code:
Code:
DestroySubnetList(iSubnets);
iSubnets = subnets;
iListLock.Signal();2. When SocketUdpMulticast::~SocketUdpMulticast() calls OpenHome::Os::NetworkSocketMulticastDropMembership, the latter method throws a NetworkError which isn't handled anywhere in the call stack. This causes the process to exit. I fixed this by adding a try/catch for this exception around the call from ~SocketUdpMulticast(). As the socket's being destroyed anyway, this network error shouldn't matter. Here's the code:
Code:
SocketUdpMulticast::~SocketUdpMulticast()
{
LOGF(kNetwork, "> SocketUdpMulticast::~SocketUdpMulticast\n");
try {
OpenHome::Os::NetworkSocketMulticastDropMembership(iHandle, iInterface, iAddress);
}
catch (NetworkError&) { // ignore read error from socket
}
LOGF(kNetwork, "< SocketUdpMulticast::~SocketUdpMulticast\n");
}3. In DviServer::SubnetListChanged, there's a minor problem with signed/unsigned integers. For the loops that decrement the index, the index variable needs to be signed to ensure correct termination of the loop when the index variable decreases from 0 to -1. In the following code, I have changed the lines marked with **:
Code:
if (current != NULL) {
** TInt i;
// remove servers whose interface is no longer available
** for (i=(TInt)iServers.size()-1; i>=0; i--) {
DviServer::Server* server = iServers[i];
if (server->Interface() != current->Address()) {
delete server;
iServers.erase(iServers.begin() + i);
}
}
// add server if 'current' is a new subnet
if (iServers.size() == 0) {
AddServer(*current);
}
current->RemoveRef();
}
else {
std::vector<NetworkAdapter*>* subnetList = adapterList.CreateSubnetList();
const std::vector<NetworkAdapter*>& nifList = adapterList.List();
** TInt i;
// remove servers whose interface is no longer available
** for (i=(TInt)iServers.size()-1; i>=0; i--) {
DviServer::Server* server = iServers[i];
if (FindInterface(server->Interface(), nifList) == -1) {
delete server;
iServers.erase(iServers.begin() + i);
}
}
// add servers for new subnets
** for (i=0; i<(TInt)subnetList->size(); i++) {
NetworkAdapter* subnet = (*subnetList)[i];
if (FindServer(subnet->Subnet()) == -1) {
AddServer(*subnet);
}
}
NetworkAdapterList::DestroySubnetList(subnetList);
}RE: Device stack crashes when network adapter disabled - simonc - 01-09-2011 01:52 PM
Thanks for such a detailed set of reports! I've committed all 3 fixes so they'll hopefully make it onto github this evening.
Subnet changing is a known area of weakness - we have a test for the control point stack but it isn't included in automated tests and still need to write a test covering the device stack. That said, you've possibly just flushed out all the bugs these tests would have found :-).
RE: Device stack crashes when network adapter disabled - simoncn - 02-09-2011 01:56 PM
(01-09-2011 01:52 PM)simonc Wrote: Thanks for such a detailed set of reports! I've committed all 3 fixes so they'll hopefully make it onto github this evening.
Subnet changing is a known area of weakness - we have a test for the control point stack but it isn't included in automated tests and still need to write a test covering the device stack. That said, you've possibly just flushed out all the bugs these tests would have found :-).
Test cases are important, but they never catch everything! It wouldn't be very easy to have an automated test for disabling an adapter. I found these problems by noticing that suspending and resuming my laptop caused the device stack to crash
