Corrupted singleton data using CxxTest

This is a weird problem and I'm not sure what to make of it.

I have something like the following:

struct Parms
{
    const std::string value1;
    const std::string value2;

    std::string parm1;
    std::string parm2;

    Parms() : parm1(value1), parm2(value1) {}

    s开发者_开发百科tatic const Parms& getDefaults()
    {
        static Parms defaults;
        return defaults;
    }
};

Which I generally use like so:

Parms myParms = Parms::getDefaults();
myParms.parm1 = "crap";
functionThatNeedsParms(myParms);

Pretty straightforward. This has never caused me any headaches, until I started trying to write unit tests that use this code, using CxxTest. I have two test suite classes in different files, and when I run them individually, everything is great.

When I run them together, I see two bad things. First, the whole thing core dumps trying to double free the static defaults variable. Secondly, if I look at the contents of defaults some time before it dies, but after I've started using it, the static const std::strings that are in there are corrupted (some letters have randomly changed, though it is always the same on every run).

What is going on?

---

double free and core dump

I think I can explain the "double free and core dump" issue that you are having. I recently encountered the same thing and it sounds like you are doing the same thing I did.

From you description you said that when you "run them separately" they work fine but if you "run them together" you get the double free/core dump issue.

I found this to occur if the same global is declared twice.

In my case I had class foo, in one file I had a global class foo gFoo; and in a different file I had a global class foo gFoo;. (Yeah this sounds stupid, actually I was linking against a file X.cxx as well as a shared library that also included X.cxx -- the results where essentially the same.)

Now, I would have expected a compiler complaint about this, but apparently there are flags to enable or disable this check, and the code compiled fine. But when the program was terminating and calling all of its destructors, it called gFoo's destructurs twice and gave me the double free message along with a core dump.

Given that you stated it works fine independently but fails when combined, I'm betting you have the global defined in two separate files, and it works fine when they are compiled by themselves, but when you combine them to make a single test, you probably have the global declaration happening twice.

Check that out.

---

static variables in C and C++ are not thread safe. That means that race conditions (bad thing) can happen if two threads try to access your singleton object. One approach to fix your problem is to use Thread Local Storage. This is supported by the pthreads library and also some compilers directly support thread local storage.

The alternative, if your singleton MUST be global to all threads, is to provide lock to ensure that only one thread can access your data at a time.

Since however, the problem arises only in unit testing. I would suggest to not run multi-threaded unit tests unless you intend to use your singleton in multiple threads.

---

This is highly dependent on the compiler and platform you are using and without actually seeing the tests I can only guess of what is going on.

I see some misconceptions in your code:
1) You are missing a copy operator and copy constructor You are copying the instances that contain std::string which might be implemented using reference counting. The reference counting is implemented in overloaded copy constructor/operator of std::string but these might not get called from the implicitly generated copy constructor of your class and therefore causing double freed memory and other nasty things. The copy operator/constructor should look like this:

// Copy constructor
Parms(const Parms& oth)  { parm1 = oth.parm1; parm2 = oth.parm2; }

// Copy operator
Parms& operator= (const Parms& oth)  { 
  if (&oth == this)  // Check for self-assignment
    return *this;
  parm1 = oth.parm1;
  parm2 = oth.parm2;
  return *this;
}

2) I don't quite understand the presence of value1 and value2. It seems you never initialize them, you just use them in the default constructor to copy their (empty) content into parm1 and parm2. You can avoid this completely as parm1 and parm2 are initialized automatically to an empty string when called.

3) You do not need to use the singleton pattern here. The getDefaults() method could be implemented as follows:

static Parms getParms() { return Parms(); }

The singleton pattern is meant for classes that only have one instance through the whole program run which doesn't seem to be the case of your class.

This way you can safely use the getParms() function from multiple threads and a smart compiler will optimize out the implied additional copy.