Skip to main content
gamedev.net
🔒 Locked

Weird static initialization problem

Started by Nairou 17 replies 2.9k views
Original Post
Nairou
Nairou
Okay, I know that static initialization is a problem once you start dealing with libraries, and I've just recently split up my project into multiple static libraries. But I don't quite understand the particular problem I'm getting. I don't see how this could occur. Here's a simplified version of the code in question: Settings.lib:
(((settings.h)))

#include <map>
#include <string>

class cvar;
class settings
{
public:
	void	registerCvar( const std::string& name, cvar* newCvar );
	// ...

private:
	static std::map<std::string,cvar*> cvarRegistry;
};


(((settings.cpp)))

#include "settings.h"

void settings::registerCvar( const std::string& name, cvar* newCvar )
{
	this->cvarRegistry[name] = cvar;	// <-- ACCESS VIOLATION
}

// Static member initialization
std::map<std::string,cvar*> settings::cvarRegistry;
Common.lib
(((cvar.h)))

class cvar
{
public:
	cvar( const char *cvarName );
	// ...
};


(((cvar.cpp)))

#include "settings.h"

cvar::cvar( const char *cvarName )
{
	// ...
	settings cvarSettings;
	cvarSettings.registerCvar( cvarName, this );
}
When both of these classes were part of the same project, not in separate libraries, it worked fine. And when I make settings::cvarRegistry non-static, it works fine. But as it is, in separate libraries, I end up with an access violation in settings::registerCvar() when It's trying to access the std::map. When I trace through it in the debugger, it appears that the map hasn't even been created, settings::cvarRegistry is just zero. But I don't understand how this could be since the cvar class is creating an instance of the settings class before using it. Shouldn't that initialize the whole class before use, including the static member? What am I missing here?
dmail
dmail
Before dealing with your problem, I would ask the question why should the map be static?
Nairou
Nairou
dmail:
It is static because I want it to be shared among all instances. The map is sort of a central repository of cvars that get registered, and any instance of settings should be able to access it. Originally, the settings class was a global shared object, but I'm trying to avoid using globals in that way.

DevFred:
I've read that a few times, and while I mostly understand it, the part that trips me up in my implementation is the fact that the class containing the static member variable is being instantiated by the same code that is trying to use it.

dmail
dmail
Quote:
Original post by Nairou
dmail:
It is static because I want it to be shared among all instances. The map is sort of a central repository of cvars that get registered ...

You have just basically described what a static instance is not why.
Quote:
and any instance of settings should be able to access it.

Yes but why have you made this design decision and why should the instance not be local to the class? Lets say you want a settings instance which is passed to a window factory or you want multiple windows and multiple setting classes. Why should all instances share the static map?
Jan-Lieuwe
Jan-Lieuwe
Why isn't registerCvar a static function? Since the map is static you shouldn't reference it through 'this'.

class cvar;class settings{public:	static void	registerCvar( const std::string& name, cvar* newCvar );	// ...private:	static std::map<std::string,cvar*> cvarRegistry;};cvar::cvar( const char *cvarName ){	// ...	settings::registerCvar( cvarName, this );}

Nairou
Nairou
Quote:
Original post by dmail
Quote:
and any instance of settings should be able to access it.

Yes but why have you made this design decision and why should the instance not be local to the class? Lets say you want a settings instance which is passed to a window factory or you want multiple windows and multiple setting classes. Why should all instances share the static map?

Well, my plan was to have a single point to hold all game settings. The game starts up, cvars register themselves, and then all settings are loaded from disk to apply settings to those cvars. Same for when the game shuts down and the cvar data is saved to disk. In addition, having all cvars in a central map means I can search through them, making it easy to type a cvar into a console and view/edit values on the fly. If this map isn't shared, then I lose the ability to sort through them all, and I either have to have each individual instance load/save the few cvars that got registered to it, or outright lose the cvar registrations once the settings instance goes out of scope.

Nairou
Nairou
Quote:
Original post by Jan-Lieuwe
Why isn't registerCvar a static function? Since the map is static you shouldn't reference it through 'this'.


The map isn't being referenced through 'this', the cvar instance is. The cvar instance passes a pointer to itself, to be inserted into the static map.
Jan-Lieuwe
Jan-Lieuwe
Quote:
Original post by Nairou
Quote:
Original post by Jan-Lieuwe
Why isn't registerCvar a static function? Since the map is static you shouldn't reference it through 'this'.


The map isn't being referenced through 'this', the cvar instance is. The cvar instance passes a pointer to itself, to be inserted into the static map.



I meant:

void settings::registerCvar( const std::string& name, cvar* newCvar ){	this->cvarRegistry[name] = cvar;	// <-- ACCESS VIOLATION}


registerCvar should be a static function and you should ommit 'this->', because cvarRegistry is a static member.
Nairou
Nairou
Quote:
Original post by Jan-Lieuwe
I meant:

void settings::registerCvar( const std::string& name, cvar* newCvar ){	this->cvarRegistry[name] = cvar;	// <-- ACCESS VIOLATION}


registerCvar should be a static function and you should ommit 'this->', because cvarRegistry is a static member.

Oh, I see what you're saying. I hadn't considered making that function static, I'll have to try that. I'm curious though what difference that would make? What does making it a static function change in this case?
rip-off
rip-off
You have a class that has no state. Consider how useful it actually is. It could be replaced by a namespace of free functions.
blog | twitter
Nairou
Nairou
Yeah it just hit me how that would work. No need to ever create an instance of the class in that case. Very weird. I can't decide if that is the way to go, or if it's an indication that I need to refactor the class...
Jan-Lieuwe
Jan-Lieuwe
Quote:
Original post by Nairou
Quote:
Original post by Jan-Lieuwe
I meant:

void settings::registerCvar( const std::string& name, cvar* newCvar ){	this->cvarRegistry[name] = cvar;	// <-- ACCESS VIOLATION}


registerCvar should be a static function and you should ommit 'this->', because cvarRegistry is a static member.

Oh, I see what you're saying. I hadn't considered making that function static, I'll have to try that. I'm curious though what difference that would make? What does making it a static function change in this case?


A static member function won't receive the this pointer as its first (hidden) parameter. You also don't need an instance of the class to call one of its static methods.

Creating an instance of a class with only static members still will use up 4 bytes (on a 32 bit machine), because C++ doesn't allow sizeof(class) == 0.

So basically you're better of with a namespace with global functions + variables. The compiler will generate the exact same code.

You could also take the 'singleton-route'. This will allow you to more easily arrange the order of construction, which was your main problem to start with.
dmail
dmail
Quote:
Original post by Nairou
Quote:
Original post by dmail
Quote:
and any instance of settings should be able to access it.

Yes but why have you made this design decision and why should the instance not be local to the class? Lets say you want a settings instance which is passed to a window factory or you want multiple windows and multiple setting classes. Why should all instances share the static map?

Well, my plan was to have a single point to hold all game settings. The game starts up, cvars register themselves, and then all settings are loaded from disk to apply settings to those cvars. Same for when the game shuts down and the cvar data is saved to disk. In addition, having all cvars in a central map means I can search through them, making it easy to type a cvar into a console and view/edit values on the fly. If this map isn't shared, then I lose the ability to sort through them all,
Why would you not be able to "sort though them" if it was a class member instance? You just have to inject the instance into anything which requires it instead of hiding the dependency.
Quote:
and I either have to have each individual instance load/save the few cvars that got registered to it, or outright lose the cvar registrations once the settings instance goes out of scope.

Again I do not understand why you think this, but at least you are now thinking about the design of the class as indicated by your comment.
Quote:

I can't decide if that is the way to go, or if it's an indication that I need to refactor the class...

Although I would go as far as saying is the settings class even required, but this is I feel now going way of topic.

Quote:
Original post by Jan-Lieuwe
You could also take the 'singleton-route'. This will allow you to more easily arrange the order of construction, which was your main problem to start with.
Please do not go down this route and try and make dependencies explicit instead of hiding them behind a global which is what you were trying to refactor out in the first place. I feel the problem you are really is having is trying to create a quick fix to remove the global instance of a settings class when as I think you now realise is your current solution or a singleton instance does not gain you anything were as a redesign would bring you a clarity in your code and allow easy testing; which currently I think your code is not using.
Nairou
Nairou
Quote:
Original post by dmail
Quote:
Original post by Nairou
Well, my plan was to have a single point to hold all game settings. The game starts up, cvars register themselves, and then all settings are loaded from disk to apply settings to those cvars. Same for when the game shuts down and the cvar data is saved to disk. In addition, having all cvars in a central map means I can search through them, making it easy to type a cvar into a console and view/edit values on the fly. If this map isn't shared, then I lose the ability to sort through them all,
Why would you not be able to "sort though them" if it was a class member instance? You just have to inject the instance into anything which requires it instead of hiding the dependency.

Could you explain this a bit more? How would you "inject" the instance?

What I don't understand is how you maintain a single map of all settings without either creating a global shared instance, or having each local instance share a static member.

Quote:
Quote:
I can't decide if that is the way to go, or if it's an indication that I need to refactor the class...

Although I would go as far as saying is the settings class even required, but this is I feel now going way of topic.

Actually I came to this same conclusion, and have rewritten the cvar class a bit to absorb the functionality of the settings class, eliminating the dependency. However, the original problem still remains, it is just isolated to a single class now.

class cvar{public:	cvar( const char *cvarName );	~cvar();	// ...private:	static std::map<std::string,cvar*> registry;};

cvar::cvar( const char *cvarName ){	// ...	this->registry[cvarName] = this;}cvar::~cvar(){	this->registry.erase( cvarName );}


However, the exact same problem occurs, an access violation when accessing the static map.

The only workaround I've found so far is to pull the static map completely out of the class and wrap it in a global function (in an anonymous namespace) and call that to get the map.

namespace {	// static wrapper to control initialization order	std::map<std::string,cvar*> cvarRegistry( void )	{		static std::map<std::string,cvar*> registry;		return registry;	}}cvar::cvar( const char *cvarName ){	// ...	cvarRegistry()[cvarName] = this;}cvar::~cvar(){	cvarRegistry().erase( cvarName );}


But I'm not sold on this being the best solution...

rip-off
rip-off
Quote:

The only workaround I've found so far is to pull the static map completely out of the class and wrap it in a global function (in an anonymous namespace) and call that to get the map.

That doesn't do what you want, because you return a copy of the map. You would need to return a reference.

As for your problem - do you have any global cvar instances?
blog | twitter
Nairou
Nairou
Quote:
Original post by rip-off
That doesn't do what you want, because you return a copy of the map. You would need to return a reference.

Woops, yeah I overlooked that, thanks.

Quote:
As for your problem - do you have any global cvar instances?

Yes, and I think that's the heart of the problem. All of these cvars are global. I define them at the top of a source file, and they are accessible by any code that needs it. This is by design.

I haven't seen any documentation to confirm it, but I'm guessing that global objects and static objects are initialized at the same time, and so there's no telling which is done first, resulting in the static member of my global class not being initialized in time.

This all worked fine when all of the code was compiled together, but has become a problem ever since I split it off into a library.

Zahlman
Zahlman
Quote:
Original post by Nairou
Yes, and I think that's the heart of the problem. All of these cvars are global.


That kind of contradicts the goal of making the settings class in the first place, hmm? :)

The idea with using a class is that you open yourself to the possibility of having more than one settings object in your system, even persistently. To access their contents, you pass them around, and they each have their own state. You create cvar instances just so that you can set up a settings instance; the settings instance is basically a glorified container of cvar instances with an adapted interface and maybe extra functionality.

If you don't gain anything from doing that, then you might as well just have the whole thing global. When everything is static-ized, you don't gain anything from the class machinery except for namespacing, which is more readily accomplished with... actual namespaces. (Creating objects and letting them share a state is sometimes a decent way to add flexibility to the design - allowing you to split off the state into per-object state later. But to benefit from this approach (the Monostate pattern), you have to forget about the static-ness of the underlying data, and pass the objects around anyway even though it's currently not necessary.)
Nairou
Nairou
Quote:
Original post by Zahlman
That kind of contradicts the goal of making the settings class in the first place, hmm? :)

Not exactly. While it's true that making the cvars global means that they are automatically accessible everywhere (which is intentional), that only works at compile-time. If I have a script that tries to read the value of a cvar (or if I try to modify the cvar using an in-game console), there is no way to look up that cvar by name. Unless it registers itself upon creation in order to create a central searchable database. Hence the settings class. :)

Since starting this thread I've dissolved the settings class and moved that database into the cvar class itself, with a static lookup function, but the problem remains. The fact that the cvars are global, combined with the fact that I have to make the map (database) static to keep it centralized, seems to be creating the initialization order conflict. Right now I'm having to use a separate function to access the static map in order to force it to be initialized first, but I still feel like I'm on shaky ground with it.

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.