Wrapping a non-unique handle with additional information

Question

I have a situation where I'd like to wrap up some plain C handles into a class, with a destructor and other niceties, in C++. I would like the wrapper class to have the exact same size as the handle, so I can pass an array of the wrappers into the C api without having to allocate and translate them first.

The handle is not unique - it has some extra information, which is known at creation time - so I can't use the handle itself as a key in one big map. Instead, I'm trying to use the address of the handle as the key.

Details aren't really important on this one - it has data, savvy?

class InstanceMetadata { ... };

The header for the wrapper class:

class Instance
{
public:
    Instance(const Instance&) = delete;
    Instance(Instance&& other);
    Instance(InstanceHandle handle, InstanceMetadata metadata);
    ~Instance();

    Instance& operator=(Instance&) = delete;
    Instance& operator=(Instance&& other);

    InstanceHandle GetHandle() const;
    void ResetHandle();

private:
    InstanceHandle handle;
};

A thin wrapper around std::map, which handles synchronization and nullptrs

template<typename Handle, typename Metadata>
class MetadataCache
{
public:
    void Set(Handle& key, Metadata* value)
    {
        std::lock_guard<std::mutex> guard(mutex);
        data[&key] = value;
    }

    Metadata* Get(Handle& key)
    {
        std::lock_guard<std::mutex> guard(mutex);
        std::map<Handle*, Metadata*>::iterator it = data.find(&key);
        if (it == data.end())
            return nullptr;
        return it->second;
    }

    void Erase(Handle& key)
    {
        std::lock_guard<std::mutex> guard(mutex);
        std::map<Handle*, Metadata*>::iterator it = data.find(&key);
        if (it != data.end())
            data.erase(it);
    }

private:
    std::mutex mutex;
    std::map<Handle*, Metadata*> data;
};

The implementation for the wrapper class:

using namespace std;
using namespace vulkan;

MetadataCache<InstanceHandle, InstanceMetadata> cache;

Instance::Instance(Instance&& other) :
    handle(other.handle)
{
    other.handle = InstanceHandle();
    cache.Set(this->handle, cache.Get(other.handle));
    cache.Erase(other.handle);
}

Instance::Instance(InstanceHandle handle, InstanceMetadata metadata) :
    handle(handle)
{
    cache.Set(handle, new InstanceMetadata(metadata));
}

Instance::~Instance()
{
    delete cache.Get(handle);
    if (handle)
        vkDestroyInstance(handle, nullptr);
}

Instance& Instance::operator=(Instance&& other)
{
    delete cache.Get(handle);
    cache.Set(handle, cache.Get(other.handle));
    cache.Erase(other.handle);
    other.handle = InstanceHandle();
    return *this;
}

InstanceHandle Instance::GetHandle() const
{
    return handle;
}

void Instance::ResetHandle()
{
    delete cache.Get(handle);
    handle = InstanceHandle();
}

The default constructor for InstanceHandle creates a null handle.

I'm still fairly new to move-semantics, so the main things I would like to know are:

• Is it safe to do this? (i.e. have I missed an operator or something?)
• Is there another way to do this?
• Is there any good reason not to do this?

Keep in mind I don't really care about performance, as these objects will not be created or destroyed very often.

Answer 1

Because I also had to deal with a lot of handles, used by for example the Win32 API, I really can understand your use case.

Is it safe to do this? (i.e. have I missed an operator or something?)

I don't think so, for a lot of reasons: Let's assume the handle your library defines is an opaque type, usually a void*. So the first problem is that the C-library has no idea of C++ concepts like operator overloading. So it will not call your operator= i.e. nor will the compiler disallow to assign named instances. Furthermore the library will deference your handle, assuming it points to it's internal data structure of some kind. Also the compiler has no idea that you have given the library a instance different from handle, while translating the library code. So even compiling the library with a C++ compiler won't make a difference.

Is there another way to do this?

Although you're desire to design a nice C++ wrapper is a good idea, I would drop the idea to slip the C-library your type. I recommend to just define a .native_handle() member function or overload the operator* but then I would suggest that the name of your type does stress the wrapper nature more.

Is there any good reason not to do this?

Despite from the reasons above, the size would already change if someone would sub or base class it.