Papierkorb / bindgen

Binding and wrapper generator for C/C++ libraries
GNU General Public License v3.0
179 stars 18 forks source link

Potential premature collection of wrapped types #75

Open HertzDevil opened 4 years ago

HertzDevil commented 4 years ago

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:

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!