Skip to content

Commit

Permalink
bug 954: avoid segfault upon startup due to recursion in TimeSet code…
Browse files Browse the repository at this point in the history
… to Simulator::Schedule
  • Loading branch information
mathieu-lacage committed Dec 12, 2012
1 parent c4c1e4d commit 55e526d
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 116 deletions.
60 changes: 27 additions & 33 deletions src/core/model/nstime.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,47 +110,47 @@ class Time
inline Time ()
: m_data ()
{
TimeSet (this);
Time::Track (this);
}
inline Time(const Time &o)
: m_data (o.m_data)
{
TimeSet (this);
Time::Track (this);
}
explicit inline Time (double v)
: m_data (lround (v))
{
TimeSet (this);
Time::Track (this);
}
explicit inline Time (int v)
: m_data (v)
{
TimeSet (this);
Time::Track (this);
}
explicit inline Time (long int v)
: m_data (v)
{
TimeSet (this);
Time::Track (this);
}
explicit inline Time (long long int v)
: m_data (v)
{
TimeSet (this);
Time::Track (this);
}
explicit inline Time (unsigned int v)
: m_data (v)
{
TimeSet (this);
Time::Track (this);
}
explicit inline Time (unsigned long int v)
: m_data (v)
{
TimeSet (this);
Time::Track (this);
}
explicit inline Time (unsigned long long int v)
: m_data (v)
{
TimeSet (this);
Time::Track (this);
}
/**
* \brief Construct Time object from common time expressions like "1ms"
Expand All @@ -175,7 +175,7 @@ class Time
*/
~Time ()
{
TimeUnset (this);
Time::UnTrack (this);
}

/**
Expand Down Expand Up @@ -295,6 +295,13 @@ class Time
* in user-expected time units.
*/
static void SetResolution (enum Unit resolution);
/**
* Freeze the current time resolution. This function is called
* internally from the Simulator::Run function. After this
* function is called, calls to SetResolution will trigger
* an assert.
*/
static void FreezeResolution(void);
/**
* \returns the current global resolution.
*/
Expand Down Expand Up @@ -402,7 +409,7 @@ class Time
explicit inline Time (const int64x64_t &value)
: m_data (value.GetHigh ())
{
TimeSet (this);
Time::Track (this);
}
inline static Time From (const int64x64_t &value)
{
Expand Down Expand Up @@ -441,8 +448,7 @@ class Time
}

static struct Resolution SetDefaultNsResolution (void);
static void SetResolution (enum Unit unit, struct Resolution *resolution,
const bool convert = true);
static void SetResolution (enum Unit unit, struct Resolution *resolution);

/**
* Record all instances of Time, so we can rescale them when
Expand All @@ -461,30 +467,18 @@ class Time
* (and gcc 4.2) say no.
*/
typedef std::set< Time * > TimesSet;
static TimesSet * GetTimesSet ();
static TimesSet ** PeekTimesSet ();
/**
* Get the TimesSet instance.
*
* \param [in] deleteMe If true delete the TimesSet, so that it returns a null pointer ever after
*/
static TimesSet * GetTimesSet ( const bool deleteMe = false );
/**
* Helper to clean up at Simulator::Run
* Start tracking a Time instance with the TimesSet
* to be able to change the underlying value if the
* time resolution changes later
*/
static void DeleteTimesSet ();
/**
* Record a Time instance with the TimesSet
*/
static void TimeSet (Time * const time);
/**
* Remove a Time instance from the TimesSet, called by ~Time()
*/
static void TimeUnset (Time * const time);


static void Track (Time * const time);
/**
* Convert existing Times to the new unit.
* We do not need to track a Time instance anymore
*/
static void ConvertTimes (const enum Unit unit);
static void UnTrack (Time * const time);

friend bool operator == (const Time &lhs, const Time &rhs);
friend bool operator != (const Time &lhs, const Time &rhs);
Expand Down
4 changes: 3 additions & 1 deletion src/core/model/simulator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,9 @@ void
Simulator::Run (void)
{
NS_LOG_FUNCTION_NOARGS ();
GetImpl ()->Run ();
SimulatorImpl *impl = GetImpl ();
Time::FreezeResolution();
impl->Run ();
}

void
Expand Down
122 changes: 40 additions & 82 deletions src/core/model/time.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
*/
#include "nstime.h"
#include "abort.h"
#include "log.h"
#include "global-value.h"
#include "enum.h"
#include "string.h"
Expand All @@ -31,8 +30,6 @@
#include <cmath>
#include <sstream>

NS_LOG_COMPONENT_DEFINE ("Time");

namespace ns3 {

Time::Time (const std::string& s)
Expand Down Expand Up @@ -84,46 +81,42 @@ Time::Time (const std::string& s)
*this = Time::FromDouble (v, Time::S);
}

TimeSet (this);
Time::Track (this);
}

// static
struct Time::Resolution
Time::SetDefaultNsResolution (void)
{
NS_LOG_FUNCTION_NOARGS();
struct Resolution resolution;
SetResolution (Time::NS, &resolution, false);
SetResolution (Time::NS, &resolution);
return resolution;
}

// static
void
Time::SetResolution (enum Unit resolution)
{
NS_LOG_FUNCTION (resolution);
SetResolution (resolution, PeekResolution ());
}

// static
enum Time::Unit
Time::GetResolution (void)
{
NS_LOG_FUNCTION_NOARGS();
return PeekResolution ()->unit;
}

// static
void
Time::SetResolution (enum Unit unit, struct Resolution *resolution,
const bool convert /* = true */)
Time::SetResolution (enum Unit unit, struct Resolution *resolution)
{
NS_LOG_FUNCTION (unit << resolution << convert);
if (convert)
{ // We have to convert old values
ConvertTimes (unit);
TimesSet * times = GetTimesSet();
if (times == 0)
{
NS_FATAL_ERROR("It is not legal to try to set the resolution " \
"_after_ it has been frozen by Simulator::Run");
}

int8_t power [LAST] = { 15, 12, 9, 6, 3, 0};
for (int i = 0; i < Time::LAST; i++)
{
Expand Down Expand Up @@ -157,37 +150,49 @@ Time::SetResolution (enum Unit unit, struct Resolution *resolution,
resolution->unit = unit;
}

// static
Time::TimesSet **
Time::PeekTimesSet (void)
{
static TimesSet *times = new TimesSet();
return &times;
}

// static
Time::TimesSet *
Time::GetTimesSet ( const bool deleteMe /* = false */ )
Time::GetTimesSet ()
{
static TimesSet * times = new TimesSet;

if (deleteMe)
{
NS_LOG_LOGIC ("deleting TimesSet");
if (times)
{
delete times;
}
times = 0;
}

return times;
TimesSet **ptimes = PeekTimesSet();
return *ptimes;
}

// static
void
Time::DeleteTimesSet ()
Time::FreezeResolution (void)
{
NS_LOG_FUNCTION_NOARGS();
Time::GetTimesSet (true);
TimesSet * times = GetTimesSet ();
if (times == 0)
{
// We froze the resolution more than once: no big deal
return;
}

for ( TimesSet::iterator it = times->begin();
it != times->end();
it++ )
{
Time * const tp = *it;
(*tp) = tp->ToInteger (Time::GetResolution());
}

TimesSet **ptimes = PeekTimesSet();
delete *ptimes;
*ptimes = 0;
}

// static
void
Time::TimeSet (Time * const time)
Time::Track (Time * const time)
{
NS_ASSERT (time != 0);

Expand All @@ -196,30 +201,12 @@ Time::TimeSet (Time * const time)
{
std::pair< TimesSet::iterator, bool> ret;
ret = times->insert ( time);
NS_LOG_LOGIC ("\t[" << times->size () << "] recording " << time);

if (ret.second == false)
{
NS_LOG_WARN ("already recorded " << time << "!");
}
// If this is the first Time, schedule the cleanup.
if (times->size () == 1)
{
// We schedule here, after the first event has been added,
// rather than in GetTimesSet when the set is empty.
// Scheduling there creates another Time, which
// finds an empty set and schedules an event . . .
// Doing it here, the schedule creates the second Time,
// which doesn't recurse.
NS_LOG_LOGIC ("scheduling DeleteTimesSet()");
Simulator::Schedule ( Seconds (0), & DeleteTimesSet);
}
}
}

// static
void
Time::TimeUnset (Time * const time)
Time::UnTrack (Time * const time)
{
NS_ASSERT (time != 0);
TimesSet * times = GetTimesSet ();
Expand All @@ -229,37 +216,8 @@ Time::TimeUnset (Time * const time)
"Time object " << time << " registered "
<< times->count (time) << " times (should be 1)." );

TimesSet::size_type num = times->erase (time);
if (num != 1)
{
NS_LOG_WARN ("unexpected result erasing " << time << "!");
NS_LOG_WARN ("got " << num << ", expected 1");
}
else
{
NS_LOG_LOGIC ("\t[" << times->size () << "] removing " << time);
}
}
}

// static
void
Time::ConvertTimes (const enum Unit unit)
{
NS_LOG_FUNCTION_NOARGS();
TimesSet * times = GetTimesSet ();
NS_ASSERT_MSG (times != 0, "No Time registry. Time::SetResolution () called mare than once?");

for ( TimesSet::iterator it = times->begin();
it != times->end();
it++ )
{
Time * const tp = *it;
(*tp) = tp->ToInteger (unit);
times->erase (time);
}

NS_LOG_LOGIC ("logged " << GetTimesSet ()->size () << " Time objects.");
GetTimesSet (true);
}

std::ostream&
Expand Down

0 comments on commit 55e526d

Please sign in to comment.