std::vector element lifetime issue

[souk4711]

Case

1. a c++ api returns an instance of std::vector
2. a ruby function returns a reference to an element of the std::vector
3. use the ruby function, the return value contains invalid data (and may cause a crash when the C++ side uses pointers)

---

Reproduce

c++ code

struct Point{
    int x() const { return m_x; }
    int y() const { return m_y; }

    int m_x, m_y;
};

extern "C" void Init_qtcore()
{
    return detail::cpp_protect([] {
        Class rb_cPoint = define_class<Point>("Point")
            .define_method("x", &Point::x)
            .define_method("y", &Point::y);

        define_global_function("get_point_list", []() -> std::vector<Point> {
            std::vector<Point> vec;
            vec.push_back(Point { 1, 2 });
            return vec;
        });
    });
}

ruby code

irb(main):001> GC.disable
=> false
irb(main):002* def find_element
irb(main):003*   get_point_list[0]
irb(main):004> end
=> :find_element
irb(main):005> e = find_element
=> #<Point:0x00007f351c366ed0>
irb(main):006> "(#{e.x}, #{e.y})"
=> "(1, 2)"

irb(main):007> GC.enable; GC.start # the point_list destroyed ?
=> nil
irb(main):008> "(#{e.x}, #{e.y})"  # invalid data
=> "(348141680, 21847)"

---

Fix

• the point list should lives longer than the elements.
• or return a copy of element ?
• or add a note to the document std support, recommanding users use Point#dup when necessary ?

[cfis]

Sure, that code will definitely crash. You created a container and let it get garbage collected, freeing all of its items. Since you maintained a reference to one of the items, the reference is invalid.

Read this, it will explain the problem and how to solve it (your example is the same as the Database/Column example):

https://ruby-rice.github.io/4.x/bindings/memory_management/?h=memory#ruby-to-c

The easiest solution is just keep a reference to the vector.

But std::vector, and the other container classes, should be updated by adding Return().takeKeepAlive to methods that return references to contained items (such as operator[]).

[souk4711]

The easiest solution is just keep a reference to the vector.

yes, and another solution is creating a copy on ruby side, e.g. get_point_list[0].dup.

but i think, the end user should not care about this stuff, so it is a bug of Std::Vector.

But std::vector, and the other container classes, should be updated by adding Return().takeOwnership to methods that return references to contained items (such as operator[]).

if i understand correctly, in rice lib, only one object can take the ownership. here, the container has the ownership of items, using Return().takeOwnership() sounds not a proper solution.

also, i encountered this issue when using the #each method, no Return() argument can be passed.

---

an easy but ugly way to resolve this issue is retrun by value, not return by reference. but according to https://ruby-rice.github.io/4.x/stl/vector/ , having two disconnected copies of data, one in C++ and one in Ruby, is usually undesirable.

[cfis]

Sorry, I meant operator[] should use Return.keepAlive(). It is designed to fix this exact problem and is an easy fix.

Rice cannot return values from a vector by value for a lot of reasons:

• Some C++ objects can't be copied (code won't compile)
• std::vector operator() returns by reference, it is not for Rice to change its API
• Copying could be very expensive
• Copying creates two disconnected items like you say which I think would be very surprising behavior to users and opposite of what Ruby arrays do.