bindgen icon indicating copy to clipboard operation
bindgen copied to clipboard

Potential premature collection of wrapped types

Open HertzDevil opened this issue 3 years ago • 0 comments

Consider these snippets:

struct Inner {
  Inner(int x) : x(x) { }

  int x;
  // other members
};

struct Outer {
  static Outer *new_no_gc() { return new Outer; }

  Outer() { last_ = this; }

  Inner *inside = nullptr;
  // other members

  static Outer *last_;
  static Outer *last() { return last_; }
};

Outer *Outer::last_ = nullptr;
Test::Outer.new_no_gc.inside = Test::Inner.new(7)
GC.collect
puts Test::Outer.last.inside.x # => ???

Boehm GC's readme states:

Any objects not intended to be collected must be pointed to either from other such accessible objects, or from the registers, stack, data, or statically allocated bss segments.

The Test::Outer and Test::Inner Crystal wrappers are always GC-enabled, so they should be collected as no Crystal variables point to them. This leaves only the actual Outer and Inner instances on the C++ side. Now the Inner instance:

  • is GC-enabled (because the generated _CONSTRUCT function invokes the UseGC allocator);
  • is pointed to from Outer::last_, but this doesn't make it accessible because Outer::last_ itself is unmanaged (the C++ wrapper for Outer::new_no_gc simply forwards the returned pointer);
  • is no longer pointed to from the managed Test::Inner instance.

Thus, it too will be collected despite being indirectly traceable through Outer::last_->inside. The last line therefore refers to a potentially deleted object, and I managed to get this snippet to crash by enlarging the Inner struct and repeating the collection step multiple times.

In this particular case, we could simply add UseGC to all occurrences of the default new operator, or use GC_malloc_uncollectable for Outer instead (the instance itself will be unmanaged, but its inside member is considered by the GC). This is clearly not possible if only the library and header files are available to Bindgen.

Qt5.cr is also prone to this issue. Example:

QStatusBar *QMainWindow::statusBar() const
{
    QStatusBar *statusbar = d_func()->layout->statusBar();
    if (!statusbar) {
        QMainWindow *self = const_cast<QMainWindow *>(this);
        statusbar = new QStatusBar(self); // unmanaged
        statusbar->setSizePolicy(QSizePolicy::Ignored, QSizePolicy::Fixed);
        self->setStatusBar(statusbar);
    }
    return statusbar;
}
window : Qt::MainWindow
window.status_bar.add_widget(Qt::Label.new "123")
# the constructed QLabel will be collected!

HertzDevil avatar Aug 17 '20 17:08 HertzDevil