Thread Rating:
  • 0 Votes - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Device stack crashes when network adapter disabled
30-08-2011, 09:57 PM
Post: #1
Device stack crashes when network adapter disabled
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);
}
Unfortunately the processing of RunCallbacks calls back recursively to NetworkAdapterList::CurrentAdapter() whch attempts to lock the list again, causing the assertion failure.

I fixed this by moving the code:
Code:
DestroySubnetList(iSubnets);
iSubnets = subnets;
iListLock.Signal();
from the end of the method to immediately before the if (subnetsChanged) test. This seems to be safe as the adapter list is no longer being modified after this point in the method.

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);
}
Find all posts by this user
01-09-2011, 01:52 PM
Post: #2
RE: Device stack crashes when network adapter disabled
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 :-).
Find all posts by this user
02-09-2011, 01:56 PM
Post: #3
RE: Device stack crashes when network adapter disabled
(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 Sad
Find all posts by this user


Forum Jump: