Problem with ns3::Timer Scheduling

[Víctor Saiáns Vázquez]

Hello everyone,

I am implementing a new module for NS-3, and I am trying to create a table with the IPs of the nodes from which my module receives packets, in order to estimate the number of nodes in the region.

My first idea was to make a list of structures like this:

        struct HostEntry {
            Ipv4Address ip;
            Timer timerLifeHostEntry;
        };

        std::list<HostEntry> m_hostsTable;

And then create a function to put a new entry into the list starting the timer (which will be re-started when the module receive a new packet from the same host), and another function to quit a certain IP from the list:

    bool VirtualLayerPlusNetDevice::PutHost(Ipv4Address ip) {
        HostEntry hostEntry;

        hostEntry.ip = ip;
        hostEntry.timerLifeHostEntry = Timer(Timer::CANCEL_ON_DESTROY);
        hostEntry.timerLifeHostEntry.SetFunction(&VirtualLayerPlusNetDevice::TimerLifeHostEntryExpire, this);
        hostEntry.timerLifeHostEntry.SetArguments(ip);
        hostEntry.timerLifeHostEntry.SetDelay(TIME_LIFE_HOST_ENTRY);

        for (std::list<HostEntry>::iterator i = m_hostsTable.begin(); i != m_hostsTable.end(); ++i) {
            if (i->ip.IsEqual(ip)) {
                i->timerLifeHostEntry.Cancel();
                i->timerLifeHostEntry.Schedule();
                NS_LOG_INFO(ip << " ya estaba en la HostTable, reiniciamos su timerLife...");

                return false;
            }
        }
        m_hostsTable.push_back(hostEntry);
        m_hostsTable.back().timerLifeHostEntry.Schedule();
        return true;
    }

    bool VirtualLayerPlusNetDevice::QuitHost(Ipv4Address ip) {
        for (std::list<HostEntry>::iterator i = m_hostsTable.begin(); i != m_hostsTable.end(); ++i) {
            if (i->ip.IsEqual(ip)) {
                i->timerLifeHostEntry.Cancel();
                m_hostsTable.erase(i);
                return true;
            }
        }
        return false;
    }

    void VirtualLayerPlusNetDevice::TimerLifeHostEntryExpire(Ipv4Address ip) {
        NS_LOG_INFO("Timer LifeHostEntry expirado en el nodo " << m_node->GetId() << ", eliminando host " << ip << " de la HostTable...\n");
        QuitHost(ip);
    }

The second time that the module receive a packet from a node (two packets from the same IP) the program crashes trying to re-schedule the Timer (Segmentation fault). I don't understand while I can't call Cancel() and next Schedule() if the timer has been configured previously when the entry was created.

Anybody can help me about this?

Thanks in advance.

Víctor

[Tommaso Pecorella]

Hi Victor,

it should be possible. Indeed it IS possible, since the same thing is being used elsewhere in the code.

Can you check that the timer has been stopped for real ? I.e., use IsRunning (if I remember the function name).

Cheers,

T.

[Víctor Saiáns Vázquez]

Hello Tommaso,

thanks for your reply, I have checked if the Timer was running (IsRunning()) previously to Cancel it (although it shouldn't matter, due to the fact that the Cancel() function does nothing in the case in which there isn't any event scheduled) and it was running, and I have checked too if the Timer wasn't running previously to call Schedule() (!IsRunning()) (In order to check if it was really canceled) and it wasn't running. The result is the same (Segmentation fault).

It seems like after calling Cancel() the previously configuration of the Timer was ignored and this have never past in my previous implementations. If I re-initialize the Timer's configuration after calling Cancel() it seems to work. But I have to assing newly a value to the timer ( i->timerLifeHostEntry = Timer(Timer::CANCEL_ON_DESTROY); ). I.e. the next PutHost() function works properly:

    bool VirtualLayerPlusNetDevice::PutHost(Ipv4Address ip) {

        HostEntry hostEntry;

        hostEntry.ip = ip;

        hostEntry.timerLifeHostEntry = Timer(Timer::CANCEL_ON_DESTROY);

        hostEntry.timerLifeHostEntry.SetFunction(&VirtualLayerPlusNetDevice::TimerLifeHostEntryExpire, this);

        hostEntry.timerLifeHostEntry.SetArguments(ip);

        hostEntry.timerLifeHostEntry.SetDelay(TIME_LIFE_HOST_ENTRY);

        for (std::list<HostEntry>::iterator i = m_hostsTable.begin(); i != m_hostsTable.end(); ++i) {

            if (i->ip.IsEqual(ip)) {

                NS_LOG_INFO(ip << " ya estaba en la HostTable, reiniciamos su timerLife...");

                i->timerLifeHostEntry.Cancel();

                i->timerLifeHostEntry = Timer(Timer::CANCEL_ON_DESTROY);

                i->timerLifeHostEntry.SetFunction(&VirtualLayerPlusNetDevice::TimerLifeHostEntryExpire, this);

                i->timerLifeHostEntry.SetArguments(ip);

                i->timerLifeHostEntry.SetDelay(TIME_LIFE_HOST_ENTRY);

                

                i->timerLifeHostEntry.Schedule();

                return false;

            }

        }

        m_hostsTable.push_back(hostEntry);

        m_hostsTable.back().timerLifeHostEntry.Schedule();

        return true;

    }

I don't understand why I have to completely re-configure the Timer after calling Cancel(). Do you?

Thanks in advance.

Víctor

[Tommaso Pecorella]

Hi Victor.

I can only think to one reason: copies.

You are not storing a pointer to the Timer in your list. You are storing a struct holding a timer.

This means that you are (somewhere) copying the Timer. And indeed, you are scheduling the one in the list, not the one outside:

  m_hostsTable.push_back(hostEntry);
  m_hostsTable.back().timerLifeHostEntry.Schedule();

I'm not that sure that Timers are container-savvy...

I'd also triple check that there is no stale event running. I.e., try firing your event and stopping/restarting it a couple of times. Then count how many times the timer is calling the function it should call. If the function is called more than once... ouch.

A simple workaround is to NOT use Timers and use EventId directly. I found that EventId are a bit harder to use (they don't automatically cancel themselves on destroy), but they're a bit safer to use.

Cheers,

T.

[Víctor Saiáns Vázquez]

Thanks a lot Tommaso, as usual you have been really helpful.

The problem was effectively that. I have made some changes to avoid the copies problem and now it's working like a charm.

Víctor

[Tommaso Pecorella]

HI Victor,

nice to know. Mind sending me a non-working example and a working one ?
We'll try to do some magic to have the compilation fail when stuff are not right.

Cheers,

T.

[Víctor Saiáns Vázquez]

Hello Tommaso,

as a non-working example you can take my first message, and the corrected code is this:

    struct HostEntry {
            Ipv4Address ip;
            Timer timerLifeHostEntry;
           

            HostEntry(Ipv4Address ipAddr):
            ip(ipAddr),
            timerLifeHostEntry(Timer::CANCEL_ON_DESTROY)
            {
            }
        };

    std::list<HostEntry> m_hostsTable;

    bool VirtualLayerPlusNetDevice::PutHost(Ipv4Address ip) {

       
        for (std::list<HostEntry>::iterator i = m_hostsTable.begin(); i != m_hostsTable.end(); ++i) {
            if (i->ip.IsEqual(ip)) {

                NS_LOG_INFO(ip << " ya estaba en la HostTable, reiniciamos su timerLife...");
                i->timerLifeHostEntry.Cancel();
               

                i->timerLifeHostEntry.Schedule();

                return false;
            }
        }
       
        m_hostsTable.push_back(HostEntry(ip));
        NS_LOG_INFO(ip << " added to the HostTable, there are " << m_hostsTable.size());
        std::list<HostEntry>::iterator it = m_hostsTable.end();
        it--;
        NS_LOG_INFO("Configuring Timer of the Host " << it->ip);
        it->timerLifeHostEntry.SetFunction(&VirtualLayerPlusNetDevice::TimerLifeHostEntryExpire, this);
        it->timerLifeHostEntry.SetArguments(ip);
        it->timerLifeHostEntry.SetDelay(TIME_LIFE_HOST_ENTRY);
        it->timerLifeHostEntry.Schedule();

        return true;
    }

    bool VirtualLayerPlusNetDevice::QuitHost(Ipv4Address ip) {
        for (std::list<HostEntry>::iterator i = m_hostsTable.begin(); i != m_hostsTable.end(); ++i) {
            if (i->ip.IsEqual(ip)) {
                i->timerLifeHostEntry.Cancel();

                i = m_hostsTable.erase(i);
                NS_LOG_INFO(ip << " deleted from the HostTable, there are " << m_hostsTable.size() << "\n");

                return true;
            }
        }
        return false;
    }

    void VirtualLayerPlusNetDevice::TimerLifeHostEntryExpire(Ipv4Address ip) {

        NS_LOG_INFO("Timer LifeHostEntry expired in the node " << m_node->GetId() << ", deleting host " << ip << " from the HostTable...");
        QuitHost(ip);
    }

I can send you the files with the complete code if you consider it necessary, but the percentage of code relevant to this issue is minimum. As you can see the principal change was adding a constructor to my struct in order to avoid invalid objects in the list when I add an element in a function (due to the fact that the elements declared in the function will be destructed when it ends). And now I only access to the objects of the list through iterator objects instead of access directly to the object through the back() function.

Regards

Víctor