Critique my approach to writing RAII wrappers

Started by
18 comments, last by jms bc 9 years, 7 months ago
Recently, I have played around with wrapping up a couple of handy C libraries in order to have a nicer more modern C++11 programming interface. I am aware that there can be associated problems, however I still find RAII wrappers handy in many cases.

So, the straight forward approach would be something like this (untested):
class Texture
{
public:
	Texture():
		_id{0}
	{
		glGenTextures(1, &_id);
		
		if (!*this)
		{
			throw std::runtime_error("Failed to acquire texture.");
		}
	}
	
	~Texture()
	{
		if (*this)
		{
			glDeleteTextures(1, &_id);
		}
	}
	
	explicit operator bool() const
	{
		return _id != 0;
	}
	
	
	// Non-Copyable
	Texture(Texture const& other) = delete;
	Texture& operator=(Texture const& other) = delete;
	
	// Moveable
	Texture(Texture&& other):
		_id{0}
	{
		swap(*this, other);
	}
	Texture& operator=(Texture&& other)
	{
		swap(*this, other);
		return *this;
	}
	
	// Wrapper Methods
	//void bind();
	//void image(...);
	//voud subImage(...);
	
private:
	GLuint _id;
	
	friend void swap(Texture& lhs, Texture& rhs)
	{
		using std::swap;
		swap(lhs._id, rhs._id);
	}
};

Initially this works fine, but it tries to do things at the same time: provide wrapper methods and manage lifetime. This becomes a problem when you receive a handle from the C api, whose lifetime is already managed in some other way. In that case, you cannot use the wrapper methods. For example, this isn't possible:
Texture getBoundTexture()
{
	GLuint boundTexture = 0;
	glGetIntegerv(GL_TEXTURE_BINDING_2D, (GLint*) &boundTexture);
	return {boundTexture};
}

Even if there was a matching constructor, this code would be buggy since now there would be two RAII objects managing one lifetime. When one of them is destroyed, the resource gets freed and the second destructor results in double deletion. Really, it should be possible to distinguish between objects that manage lifetimes and those that don't. Well, you might say there already is such a thing: smart pointers! I could create my Texture object on the heap and use smart pointers to manage its lifetime. However, there is really no need for creating objects on the heap, if instead we generalize smart pointers. In fact, this has been proposed. However, it is not part of the standard library yet and - what I think is even worse - it's not easily possible to associate wrapped methods with the handle.

So, instead I have come up with the following approach (which I am sure has its own problems, that's why I am looking for feedback):

First, lifetime management is encapsulated in a separate class (similar to the proposed unique_resource):
template<typename T>
class Unique
{
public:
	template<typename... Args>
	Unique(Args&&... args):
		_value{}
	{
		T::New(_value, std::forward<Args>(args)...);
		
		if (!_value)
		{
			throw std::runtime_error("Failed to acquire resource.");
		}
	}
	
	~Unique()
	{
		if (_value)
		{
			T::Delete(_value);
		}
	}
	
	Unique(Unique const& other) = delete;
	Unique& operator=(Unique const& other) = delete;
	
	Unique(Unique&& other):
		_value{}
	{
		swap(*this, other);
	}
	Unique& operator=(Unique&& other)
	{
		swap(*this, other);
		return *this;
	}
	
	T& 			operator*() 		{ return _value; }
	T const& 	operator*() const 	{ return _value; }
	
	T* 			operator->() 		{ return &_value; }
	T const* 	operator->() const 	{ return &_value; }
	
private:
	T _value;
	
	friend void swap(Unique& lhs, Unique& rhs)
	{
		using std::swap;
		swap(lhs._value, rhs._value);
	}
};

And the wrappers look like this:
class Texture
{
public:
	typedef GLuint Handle;
	
	static void New(Texture& object)
	{
		glGenTextures(1, &object._handle);
	}
	
	static void Delete(Texture& object)
	{
		glDeleteTextures(1, &object._handle);
	}
	
	Texture(): _handle{0} {}
	Texture(Handle const& handle): _handle{handle} {}
	
	Handle const& get() const { return _handle; }
	
	explicit operator bool() const
	{
		return _handle != 0;
	}
	
	// Wrapper Methods
	//void bind();
	//void image(...);
	//voud subImage(...);
	
private:
	Handle _handle;
	
	friend void swap(Texture& lhs, Texture& rhs)
	{
		using std::swap;
		swap(lhs._handle, rhs._handle);
	}
};

The usage could be as follows (artificial example):
{
	Texture bound = getBoundTexture(); // Imlementation as before, now works.

	Unique<Texture> managed;
	
	//Setup texture etc.
	//managed->image(...);
	
	managed->bind();
	
	// Draw Something

	bound.bind();
}

So, the wrappers are like a plain pointer, whereas if you want their lifetime managed, you should use Unique<Texture>. At the end of the block, the manage texture is destroyed, whereas the plain one is untouched. Of course, it would also be possible to implement a Shared<Texture> in the future.

Anyway, I am curious to hear your thoughts. Please critique away...
Advertisement

RAII does not mean "allocate resources".

RAII means "initialized".

It looks like you are starting to understand that in your original vs updated post, but don't make the final leap.

RAII typically means doing the minimum possible work to get the object in a known state. When working in C, RAII means calling bzero() on new structures. In C++ that means making sure all the member variables that matter are initialized to zero, null, or some other known state during construction, or to intentionally leave them in an indeterminate state because their value has no meaning at that point.

If you have a bunch of member variables and never initialize them to a value you do not follow RAII. When someone creates an object the variables will point to uninitialized data. The values will simply be whatever happened to be in the memory spot before it. If the memory spot already happened to contain "32", it will have the value "32". If the memory region happened to already have a value for a password, the new object value will be that password.

In your case NOT following RAII would mean leaving _id as an uninitialized value in the first example. In the second case _handle is an object so the language will automatically call the constructor behind your back even if you don't invoke it during an initializer list; hopefully it also initializes all values to a known state. Better to use an initializer list or use the new syntax for default values.

You can provide alternate constructors that connect to resources or do other work, but those should not be your default constructors.

When in doubt, look at what the Standard Library does. When you create a vector with a default constructor it is empty. When you create a string with a default constructor it is empty. When you default-construct an fstream it is not connected to a file. There are alternate constructors to fill a vector, point to an existing string, and spend time to fire up the disk, search the file system, and connect to a file, but these are not the default constructors.

In your original version and your modified version you are requiring resources beyond those needed for RAII.

In your case, any instance of a Texture object MUST allocate a graphics resource. If I were to attempt to create an array of texture objects for any purpose whatsoever I am obligated to also allocate OpenGL resources. If i were to, for example, make an array of 50 Texture objects so I could do something interesting, there is absolutely no way I can create these texture objects without also having GenTextures() run.

Even if I'm not going to actually render the textures GenTextures() is run.

Even if I'm pre-allocating an array during game startup before OpenGL is even configured, GenTextures() is run.

In your updated version you still are equating RAII with resource allocation.

Initializtion does not mean resource allocation. Initialization means a minimal empty valid state.

RAII does not mean "allocate resources".

Whaaaat?

"Resource allocation is instantiation" does not mean resource allocation is instantiation? What kind of wacky newspeak are we dealing with here?

Perhaps the simple answer is that if you can not control the lifetime of a resource, you can not use RAII to wrap it. It's just not the appropriate tool for the job.

Stephen M. Webb
Professional Free Software Developer

RAII does not mean "allocate resources".

Whaaaat?

"Resource allocation is instantiation" does not mean resource allocation is instantiation? What kind of wacky newspeak are we dealing with here?

Perhaps the simple answer is that if you can not control the lifetime of a resource, you can not use RAII to wrap it. It's just not the appropriate tool for the job.


RAII stands for Resource Acquisition Is Initialization. Allocation doesn't come into the picture.

As an example of something that follows the RAII pattern but doesn't allocate, I offer the simple lock_guard class:


void MyFunc()
{
  std::lock_guard<std::mutex> myLock(MyMutex); // lock_guard locks the mutex in the constructor
  // Do stuff
  // lock_guard calls unlock in destructor when the scope is exited - either normally or via exception
}
Ref-counted smart pointers are another example of something that doesn't allocate or deallocate. They add a ref to an object in the constructor, and then decrement the ref count in the destructor. Granted, de-allocation can happen if the ref count goes to 0, but the important part of RAII is to make sure that things that do cleanup get a chance to do cleanup, no matter how scope is exited.

Heck, I've written tiny structs with function calls in constructors and the corresponding cleanup calls in the destructors just so that I don't have to remember to put the cleanup code every place the function can exit (an impossible task with exceptions).

Initializtion does not mean resource allocation. Initialization means a minimal empty valid state.


First, I wanted to highlight that I intentionally did not implement RAII in the Texture class. It is merely wraps a plain handle and adds convenience wrappers. I might as well have called it TextureHandle or something like that. The RAII comes into play when you use this class in conjunction with Unique, which puts the object into a valid initialised state on construction. To sum it up: int* is not RAII, but std::unique_ptr<int> is RAII. Similarly, Texture is not RAII, but Unique<Texture> is RAII.

Edit: Upon re-reading your post, I may have misinterpreted it the first time. Are you suggesting that it should be possible to create a Unique<Texture> without creating an OpenGL texture object? I must say I tend to agree. I suppose creating it in an empty state and having a makeUnique method which calls Texture::New similar to those in the standard library would be preferable.

RAII does not mean "allocate resources".

Whaaaat?

"Resource allocation is instantiation" does not mean resource allocation is instantiation? What kind of wacky newspeak are we dealing with here?

Perhaps the simple answer is that if you can not control the lifetime of a resource, you can not use RAII to wrap it. It's just not the appropriate tool for the job.

RAII is actually a total misnomer. The reason it's used has almost nothing to do with resource allocation (or indeed initialisation or instantiation) and everything to do with resource cleanup.

I remember reading in the C++ FAQ that a more accurate acronym would be DIRR (Destruction Is Resource Reclamation), but we're kinda stuck with RAII now.

if you think programming is like sex, you probably haven't done much of either.-------------- - capn_midnight

RAII does not mean "allocate resources".

Whaaaat?

"Resource allocation is instantiation" does not mean resource allocation is instantiation? What kind of wacky newspeak are we dealing with here?

Perhaps the simple answer is that if you can not control the lifetime of a resource, you can not use RAII to wrap it. It's just not the appropriate tool for the job.

RAII is actually a total misnomer. The reason it's used has almost nothing to do with resource allocation (or indeed initialisation or instantiation) and everything to do with resource cleanup.

I remember reading in the C++ FAQ that a more accurate acronym would be DIRR (Destruction Is Resource Reclamation), but we're kinda stuck with RAII now.

I vote this up 10 more times if I could. RAII is about cleanup far more than it is about initialization.


AII is actually a total misnomer. The reason it's used has almost nothing to do with resource allocation (or indeed initialisation or instantiation) and everything to do with resource cleanup.

That's not correct either, although you're right that it's a bit of a misnomer, and resource cleanup is the mandatory sequelae to resource acquisition in RAII.

RAII means the object invariant is ownership of a resource. Once the object has been constructed, you have the resource, and once the object has been destroyed you no longer have the resource. If at any point you do not have the resource and you have the object, the invariant has been contravened and you do not have RAII. Similarly, if you have the resource but not the object, it's not RAII.

The point of the invariant is that it's a guarantee that the object == the resource. Both ways. Always and forever.

If you are using deferred construction (as frob was discussing), you are not using RAII because it breaks the object invariant.

If you have singular objects or nullable objects (eg. smart pointers) you are not implementing RAII (although you can use std::unique_ptr to implement RAII if you remove the move operations). It breaks the invariant.

Things that do the above are valid and useful, but they're not implementing RAII. You're not using RAII just because you clean things up in your destructor: that's just basic object oriented programming.

So yes, RAII is very very much about resource acquisition on instantiation, and also about resource release on destruction. Like NVI, the tag for the idea is not a complete description of what it means but if you're not acquiring the resource on object instantiation, you're not implementing "Resource Acquisition Is [Object] Instantiation".

Stephen M. Webb
Professional Free Software Developer

Bregma: While true for some things, it is not universal.

There are many resources -- containers being the most obvious -- where "initialized" correctly means no allocations.

If you look back before RAII was a term that people thought about, the pattern was this:

// Acquisition. Gain a resource but the initial contents are whatever happened to previously be in memory
myObject = malloc(sizeof(sometype));
// Initialization. Put the resource in a known state
bzero(myObject, sizeof(sometype));

or this

// Acquisition. Gain a chunk of memory with unknown contents
myObject = malloc(sizeof(sometype));
// Initialization. Put the resource in a known state
memcpy(myObject, templateObject, sizeof(sometype), 1);

Object oriented languages improve on this with constructors that perform initialization for you. But even they are not perfect as some types do not initialize their values.

class Foo {
int x;
int y;
public:
Foo() { } // Both x and y are acquired but not intialized. Their contents could be anything.
...
}

RAII can be followed by initializing the object during construction:

Foo() : x(0), y(0) {} // RAII is followed, the object is initialized to a known state.


Yes, there are some classes where initialization absolutely requires additional allocations. But those situations in practice are extremely rare and are usually easily avoidable.

If a default constructor requires trips out for heap allocations you are probably doing something wrong. If a default constructor requires trips out to disk or across a network, you are absolutely doing something wrong. You can provide parameters for non-default constructors, but a default constructor should be instant and not consume additional resources beyond those to initialize the object to a known state.

As an example of a horrible case of default constructors doing this, I have seen beginner code where a default constructor would look in a known resource to look up the next item to load (in this case, an already open file). After reading the next line (potential round trip to disk could be quite long) it would open a file (which means the OS looks up a file name on potentially one or two disk reads, followed by another disk read to get the file). Then it would parse the file for names of sub-models and sub-textures, which in turn spawned off another series of reads and loads. All total using this default constructor required something on the order of 150 disk reads and 500 memory allocations. They were confused why allocating an array of these objects was slow.

Default constructors are used frequently in the language, and many default-constructed objects are allocated but never used. Requiring the default constructor to perform more work than simple initialization to a known state is often a serious implementation flaw.

If you're not acquiring a resource in your constructor, you're not implementing RAII. You're not (necessarily) writing bad code, you're just not implementing RAII.

If you have instantiated an object that does not acquire a resource, it is not implementing "resource acquisition is instantiation". It might be wonderful, perfect code that does exactly what you want in the best way possible. It's just not RAII. You might initialize an object to a known state, and that's grand, but if the purpose of the object is to acquire a resource, and the object can exist without acquiring that resource, it's not implementing RAII. Simply initializing an object to a known state and freeing any resources on object destruction isn't RAII, it's just general object-oriented programming with deterministic destruction.

If you use RAII, the statement "if the object exists, the resource is allocated" is always true. Also, the statement "if the resource is allocated, the object exists" is true. It is a boon to reasoning about program logic. But the point of RAII is that you never have to reason something like "the object exists, and it may or may not have acquired resource X, so...."

Standard containers are not an example of RAII. The standard smart pointers are not an example of RAII.

Stephen M. Webb
Professional Free Software Developer

This topic is closed to new replies.

Advertisement