Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential premature collection of wrapped types #75

Open
HertzDevil opened this issue Aug 17, 2020 · 0 comments
Open

Potential premature collection of wrapped types #75

HertzDevil opened this issue Aug 17, 2020 · 0 comments

Comments

@HertzDevil
Copy link
Contributor

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!
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

No branches or pull requests

1 participant