Better Singleton Support?

[libmesh]

Better Singleton Support?

[jwpeterson]

On Mar 29, 2013, at 7:10 AM, "Benjamin S. Kirk" [email protected] wrote:

Ok, the discussion in #62 and fear of the alternate implmentation got me thinking that if we had better core support for Singletons inside the library the alternative may not be so bad...

Right now the only one we really have is RemoteElem. I've introduced more in #62, so it seems maybe we need a more general way to handle these.

Ideally singletons can be created as static data inside a class, or as a static object inside a function. The latter allows you to control when the singleton is created:

const Singleton & get_singleton()
{
// some thread safety mechanism...
static Singleton singleton;
return singleton;
}
The nice thing about that is the singleton is not created until needed. Unfortunately the destruction is still a mystery (to me anyway?), and problematic if the Singleton is a reference counted object.

Yep, this solves the SIOF problem but not the destruction order problem, which can also be an issue if the destructor of one class depends on another... A case I don't believe we currently have.
Our current approach is to create singletons in LibMeshInit() and destroy them in ~LibMeshInit(). I like this, but rather than have LibMeshInit() ultimately know and manage all singletons, what if instead it contained a list of pointers to LibMeshSingleton objects or something?

class LibMeshSingeton
{
public:
void tear_down();
};
The LibMeshInit destructor loops over all objects (that were added somewhere else) and calls tear_down(); - thereby allowing a predictable and safe destruction order, while still allowing singletons to get created only at the time of first access, which in some cases could be never?

I imagine you will need multiple inheritance to do this. And how about the actual destructor of said object? Should it check to ensure that it's already been torn down?

In the RemoteElem implementation then, for example, there is a simple

class RemoteElemSingleton : public LibMeshSingleton
{
RemoteElemSingleton ()
{
// constructor adds *this to the LibMeshInit list of singletons in a thread-safe way
}

void tear_down ()
{
if (_remote_elem)
{
delete _remote_elem;
_remote_elem = NULL;
}
...
}
};
@roystgnr, @jwpeterson, @friedmud -- thoughts?

—
Reply to this email directly or view it on GitHub.

[benkirk]

On Mar 29, 2013, at 8:50 AM, "jwpeterson" [email protected] wrote:

Yep, this solves the SIOF problem but not the destruction order problem, which can also be an issue if the destructor of one class depends on another... A case I don't believe we currently have.

Ah, yes - we don't but we could... I mean, right now we are working around that sort of - the reference counting foo is a singleton that we are deriving singleton objects from...

Seems like a ubiquitous problem - anyone aware if there is a boost-ish like solution?

-Ben

[friedmud]

Withdrew my solution for now because it had a memory leak ;-) Currently fixing ;-)

[benkirk]

Seems like a ubiquitous problem - anyone aware if there is a boost-ish
like solution?

I'm not, but that doesn't mean there isn't something Boost-ish that solves
it.

• Rhys

[benkirk]

On Mar 29, 2013, at 9:16 AM, Derek Gaston [email protected] wrote:

Here's what I'm thinking:

• Static data does get destroyed (even refcounted objects) as the program ends. They get destroyed in reverse order of initialization (but you can't know the order of initialization across compilation units).

I didn't realize the reverse order was guaranteed?

• Does the order really matter? Surely the destructors of these objects won't call each other.

Yes and No (below…)

• If you really do want to do this, the easiest way is with a static instance of the Singleton base class that keeps pointers to every instance of it's children and then destructs them as it gets deleted at the end of the program. That ensures that objects are destructed in perfect reverse order (based on when they were created) across compilation units.

The only reason the order matters is because of Singletons derived from Singletons - the RemoteElem : ReferenceCountedObject business.

Although provided we can guarantee the reference counting foo is initialized before any static Elem singletons, and your reverse destruction is guaranteed, I think that is enough…?

-Ben

[friedmud]

Ok here's my solution again... but it's really not all that cool now. Basically the base class just registers each singleton automatically as they are created... and then we can delete them later in a guaranteed reverse order. This has the advantage that you won't ever create a singleton and forget to add it to the tear down list.

// Deleted in favor of my final solution

[friedmud]

You could do this without needing tearDown as well now... just use static pointers instead and store pointers instead (similar to my first solution) then delete them in reverse order.

[friedmud]

Ok - here's my final solution:

#include <vector>
#include <iostream>

class SingletonBase;
static std::vector<SingletonBase *> _singletons;

class SingletonBase
{
public:
  virtual ~SingletonBase()
  {}

  virtual void tearDown() {}

protected:
  SingletonBase()
  {
    _singletons.push_back(this);
  }
};


class SomeSingleton : public SingletonBase
{
public:
  static SomeSingleton * getSomeSingleton()
  {
    static SomeSingleton * some_singleton = new SomeSingleton;
    return some_singleton;
  }

  virtual ~SomeSingleton() { std::cout<<"Deleting SomeSingleton"<<std::endl; }

protected:
  SomeSingleton() {}
};

class AnotherSingleton : public SingletonBase
{
public:
  static AnotherSingleton * getAnotherSingleton()
  {
    static AnotherSingleton * another_singleton = new AnotherSingleton;
    return another_singleton;
  }

  virtual ~AnotherSingleton() { std::cout<<"Deleting AnotherSingleton"<<std::endl; }

protected:
  AnotherSingleton() {}
};

int main()
{
  SomeSingleton::getSomeSingleton();
  AnotherSingleton::getAnotherSingleton();

  for(unsigned int i=_singletons.size()-1; i != -1; i--)
    delete _singletons[i];
}

[benkirk]

On Mar 29, 2013, at 9:40 AM, "Derek Gaston" [email protected] wrote:

Ok - here's my final solution

I like it. If there is general agreement this would be a Good Thing then ill leave this issue open and implement/refactor after my current other feature blitz subsides.

-Ben

[jwpeterson]

On Fri, Mar 29, 2013 at 8:44 AM, Benjamin S. Kirk
[email protected]:

On Mar 29, 2013, at 9:40 AM, "Derek Gaston" [email protected]
wrote:

Ok - here's my final solution

I like it. If there is general agreement this would be a Good Thing then
ill leave this issue open and implement/refactor after my current other
feature blitz subsides.

I definitely don't agree with adding infinite loops to the code...

int main()
{
SomeSingleton::getSomeSingleton();
AnotherSingleton::getAnotherSingleton();

for(unsigned int i=_singletons.size()-1; i != -1; i--)
delete _singletons[i];

}

[benkirk]

On Mar 29, 2013, at 10:01 AM, "jwpeterson" [email protected] wrote:

I definitely don't agree with adding infinite loops to the code...

int main()
{
SomeSingleton::getSomeSingleton();
AnotherSingleton::getAnotherSingleton();

for(unsigned int i=_singletons.size()-1; i != -1; i--)
delete _singletons[i];

}

Good place for an iterator...

[jwpeterson]

On Fri, Mar 29, 2013 at 8:23 AM, Benjamin S. Kirk
[email protected] wrote:

On Mar 29, 2013, at 9:16 AM, Derek Gaston [email protected] wrote:

Here's what I'm thinking:

• Static data does get destroyed (even refcounted objects) as the program ends. They get destroyed in reverse order of initialization (but you can't know the order of initialization across compilation units).

I didn't realize the reverse order was guaranteed?

I don't think it is unless the compiler can discern the dependencies
from the class constructors. At least, that's how I read this C++ FAQ
(http://www.cs.technion.ac.il/users/yechiel/c%2B%2B-faq/construct-on-first-use-v2.html)

"If the constructors of a, b and c use ans, you should normally be
okay since the runtime system will, during static deinitialization,
destruct ans after the last of those three objects is destructed.
However if a and/or b and/or c fail to use ans in their constructors
and/or if any code anywhere gets the address of ans and hands it to
some other static object, all bets are off and you have to be very,
very careful."

John

[friedmud]

@jwpeterson Whoops! "unsigned" doh.... lol. Yeah just use an iterator... ;-)

[roystgnr]

I really like Derek's last solution as modified by either John's bug
fix or by using an iterator instead.

I've been trying to think of cons but so far my only concern is "will
this interfere with future changes to allow multiple LibMeshInit
objects to coexist" and my best estimate is "after a new static
int member and a couple simple constructor/destructor changes in
LibMeshInit, singletons would continue to work fine".

This looks good.

[benkirk]

Implemented in dde02a2