template <typename T> class singleton
{
public:
    static T* ms_singleton;

    singleton()
    {
        assert(!ms_singleton);
        long offset = (long)(T*)1 - (long)(singleton <T>*) (T*) 1;
        ms_singleton = (T*)((long)this   offset);
    }

    virtual ~singleton()
    {
        assert(ms_singleton);
        ms_singleton = 0;
    }

    static T& Instance()
    {
        assert(ms_singleton);
        return (*ms_singleton);
    }

    static T* instance_ptr()
    {
        return (ms_singleton);
    }
};

class Test
{
private:
   DWORD X;
}

typedef Test* LPTEST;

class TestManager : public singleton<TestManager>
{
public:
    TestManager();
    virtual ~TestManager();

    LPTEST  CreateTest(DWORD id);

protected:

private:
    std::unordered_map<DWORD, LPTEST> test_map;
};

LPTEST TestManager::CreateTest(DWORD id)
{
    LPTEST test = new Test;

    if (id)
    {
        test_map.insert(std::make_pair(id, test));
    }

    return (test);
}

I created a code similar to this.

when i run the below function.

LPTEST Tested = TestManager::Instance().CreateTest(61);

but "test_map.insert" gives an access violation error.

test_map.insert -> If I turn off the function it works fine.

all codes are normal. why am I having such a problem with std:map like codes.

I would be glad if you help.

CodePudding user response：

by changing the Singleton code as follows. I solved the problem.

template <typename T> class singleton
{
public:
    singleton() {}
    ~singleton() {}

    static T& Instance()
    {
        static T instance;
        return instance;
    }

    singleton(const singleton&) = delete;
    singleton& operator=(const singleton&) = delete;
};

@drescherjm , @UnholySheep Thanks for your help.

Do you think there is a code in this code that will cause trouble later? Could this code be better? How?

CodePudding user response：

You code is way too complicated, dangerous and full of undefined behavior.

Here are some of the problems:

• The whole offset computation is a complete non sense!
• C style cast should be avoid. You should always use C cast like static_cast, dynamic_cast perfectly understanding why you use such cast.
• You have a pointer to T but you never allocated any memory for such an object. As written, you mostly assume that memory is there.

In practice, with Meyer's singleton you rarely need to define such template.

Something like that is much simpler:

using TestMap = std::unordered_map<DWORD, Test>;

class TestManager
{
public:

    static TestMap &GetTestMap();

    Test *CreateTest(DWORD id);
};

// testmanager.cpp
TestMap &TestManager::GetTestMap()
{
    // Creation of the object is thread safe but if you want
    // to use it from multiple threads, you have to provide
    // your own synchronisation like mutex.
    static TestMap testMap;
    return testMap;
}

Test *TestManager::CreateTest(DWORD id)
{
    // allocate a pair and return a reference to value
    // if key already exist, return a reference to existing value.
    auto &mapValue = GetTestMap()[id];

    // as unique_ptr are stored in the map, 
    // memory managment can be simplified
    // could also use make_unique here
    mapValue.reset(new Test);

    // return a raw pointer as in your original code
    return mapValue.get();
}

Depending on the need and your level of understanding of C , that code can be improved... As written, it is simple to understand once you know some basic C 11.