Skip to content

Potential premature collection of wrapped types #75

@HertzDevil

Description

@HertzDevil

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!

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions