Skip to content

Conversation

@silvertakana
Copy link

There isn't an issue. This is just an addition for more functionality

@JonathanHiggs
Copy link
Contributor

JonathanHiggs commented Jul 6, 2022

GetRef is bugged, super easy to end up in undefined behaviour, either with a local variable that has been deleted while still in scope, or a ref that points to memory that has already been deleted (see examples below)

The issue is that passing in a T& implies that the caller still owns the memory, but you are passing that into shared_ptr which implies it is taking over ownership of the memory

struct Foo
{
    bool& deleted;
    ~Foo() { deleted = true; }
};


TEST(RefTest, RefDeletesInScopeValue)
{
    bool deleted = false;
    Foo foo { deleted };

    {
        auto ref = GetRef<Foo>(foo);
    }

    // foo is still in scope but has been deleted by the ref going out of scope
    EXPECT_FALSE(deleted);
}

TEST(RefTest, RefPointsToReclaimedMemory)
{
    bool deleted = false;
    auto returnRef = [&]() -> Ref<Foo> {
        Foo foo{ deleted };
        return GetRef<Foo>(foo);
    };

    auto ref = returnRef();

    // foo was deleted when it went out of scope so ref points to reclaimed memory
    EXPECT_FALSE(deleted);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants