c++visual-studio-2017make-shared

Can make_shared occasionally create duplicate pointers?


I'm failing some of my unit tests about 25% of the time per test. The problem is that when I create an "Entity" object, I use the std::make_shared function to create a pointer to another object. It seems that occasionally, pointers in separate Entity objects point to the same transform object. I'm not sure how this is possible. Here's the relevant code:

Entity constructor:

Entity::Entity(std::shared_ptr<Shape> i_shape) :
    shape {i_shape}
{
    transform = std::make_shared<Transform> (Vector2f(0.0f, 0.0f), Vector2f(1.0f, 1.0f), 0.0f);
}

Visual Studio 2017 Unit test code revealing the issue:

        TEST_METHOD(CircleCollisionTest2)
        {
            Entity* testCircleEnt1;
            Entity* testCircleEnt2;
            std::shared_ptr<Circle> testCircle1 = std::make_shared<Circle>(60.0f);
            std::shared_ptr<Circle> testCircle2 = std::make_shared<Circle>(60.0f);
            testCircleEnt1 = &Entity(testCircle1);
            testCircleEnt2 = &Entity(testCircle2);
            testCircleEnt1->GetTransform()->SetPostion(Vector2f(0, 0));
            testCircleEnt2->GetTransform()->SetPostion(Vector2f(200, 0));

            Assert::IsFalse(Physics::TestCollision(testCircleEnt1, testCircleEnt2));
        }

When I instantiate my Entity objects, make_shared is creating pointers to the same Transform objects. I've confirmed that they hold the same address in debug mode, so it shouldn't involve my getters or setters or anything like that.

Also, whenever this happens, the failing test takes ~150ms to complete rather than ~5ms. How can be happening roughly 25% of the time rather than 100% or 0%? I thought it could be related to the Visual studio testing framework but I replicated these results in a custom made test.

Any help would be very welcome. Thanks!


Solution

  • Your code code has undefined behavior. When you do

    testCircleEnt1 = &Entity(testCircle1);
    testCircleEnt2 = &Entity(testCircle2);
    

    You create two temporary objects and store the address of those. First, that should not compile. If it is, you need to turn up your wanring/error options. Second, temporary objects are destroyed at the end of the full expression they are in so that leaves you with two dangling pointers. What your code should be is

    TEST_METHOD(CircleCollisionTest2)
    {
        std::shared_ptr<Circle> testCircle1 = std::make_shared<Circle>(60.0f);
        std::shared_ptr<Circle> testCircle2 = std::make_shared<Circle>(60.0f);
        testCircle1->GetTransform()->SetPostion(Vector2f(0, 0));
        testCircle2->GetTransform()->SetPostion(Vector2f(200, 0));
    
        Assert::IsFalse(Physics::TestCollision(testCircle1.get(), testCircle2.get()));
    }
    

    Since you are using visual studio, you should turn on the /permissive- to enfore stronger compliance to the C++ standard.