kojix2 / LibUI

A portable GUI library for Ruby
MIT License
208 stars 10 forks source link

Abnormal termination during callback functions? #8

Closed kojix2 closed 3 years ago

kojix2 commented 3 years ago

Some examples raise segmentation fault if you click on widgets quickly. The cause is unknown.

kojix2 commented 3 years ago

This issue was looked into in detail by @kou, one of the maintainers of Fiddle. Thank you @kou for your great work!

On the Red-Data-Tools gitter (japanese)

再現しました。これはGCの問題ですね。 スピンボタンを押し続けた時はUI.spinbox_on_changedで登録したコールバックが呼ばれるのですが、libui.rbのargs.map!でFiddle::Closure::BlockCaller.newしているやつはUI.spinbox_on_changedを実行している間しかGCから保護されていないのでUI.spinbox_on_changed呼び出しが終わった後のGCで解放されてしまいます。解放されてからスピンボタンを押すと解放済みの領域を触ってしまってクラッシュします。

(deepl) It reproduced. This is a GC problem. When the spin button is held down, the callback registered in UI.spinbox_on_changed is called, but the Fiddle::Closure::BlockCaller.new in args.map! will be released by the GC after the UI.spinbox_on_changed call is finished. If you press the spin button after it is released, it will crash because it touches the area already released.

args.map!.with_index do
   callback = Fiddle::Closure::BlockCaller.new(*func.inner_functions[idx][1..2], &arg)
   @@callbacks << callback
   callback
end

とかしてGCから保護するとクラッシュしなくなります。が、↑は不要になったコールバックも維持し続けてしまうので微妙です。 gtk3 gemではウィジェットオブジェクトにコールバックの参照を持たせてウィジェットオブジェクトが解放されたらコールバックも解放されるようにしていますが、LibUIでも同じアプローチができるかはわからないです。

(deepl) If you protect it from GC, it will not crash. However, the above is a bit tricky because it also keeps callbacks that are no longer needed. In the gtk3 gem, the widget object has a reference to the callback so that when the widget object is freed, the callback is also freed, but I don't know if the same approach can be used for LibUI.

とりあえず、スピンボタンに関しては↓でも大丈夫みたいです。

(deepl) At least for the spin button, it seems to work fine.

args.map!.with_index do
  callback = Fiddle::Closure::BlockCaller.new(*func.inner_functions[idx][1..2], &arg)
  receiver = args[0]
  callbacks = receiver.instance_variable_get(:@callbacks) || []
  callbacks << callback
  receiver.instance_variable_set(:@callbacks, callbacks)
  callback
end

落ちる原因はもう一つあってそれもGCの問題です。 AreaHandlerのコールバックがGCされます。 handler.MouseEventとかはFiddleくんはそれがRubyのオブジェクトなのでどうかわからないのでGCから保護しません。なので、アプリケーション側でGCから保護しないといけません。

(deepl) There is another reason for the crash, which is also a GC problem: AreaHandler callbacks are GCed, and handler.MouseEvent is not protected from GC because Fiddle doesn't know if it is a Ruby object or not. So we need to protect it from GC on the application side.

ユーザーにやってもらうにはたとえばこうです。

(deepl) Here's how to get users to do it, for example.

handler.MouseEvent   = (handler_mouse_event = Fiddle::Closure::BlockCaller.new(0, [0]) {})
handler.MouseCrossed = (handler_mouse_crossed = Fiddle::Closure::BlockCaller.new(0, [0]) {})
handler.DragBroken   = (handler_drag_broken = Fiddle::Closure::BlockCaller.new(0, [0]) {})
handler.KeyEvent     = (handler_key_event = Fiddle::Closure::BlockCaller.new(1, [0]) {0})

ライブラリー側でやるならたとえばAreaHandlerを拡張してこんな感じです。

(deepl) If you want to do it on the library side, for example, you can extend AreaHandler to do something like this.

AreaHandler = struct [...]
class AreaHandler
  def on_mouse_crossed(&block)
     @mouse_crossed = Fiddle::Closure::BlockCaller.new(:void,
                                                      [:voidp, :voidp, :voidp],
                                                      &block)
    self.MouseCrossed = @mouse_crossed
  end
end

(deepl) Here's what the user does

使う側はこう。

 handler.on_mouse_crossed do
end
kojix2 commented 3 years ago

I think the API needs to be improved.

It's not pleasant to write Ruby code while worrying about callbacks and garbage collection.

The problem is obvious: a Ruby object called Fiddle::Closure in a C callback can be arbitrarily destroyed by garbage collection at any time, freeing up memory. To prevent this, you must always add a reference to it.

The problem is not completely solved, but the cause and solution are now clear. Close the issue.

rubyFeedback commented 3 years ago

Just noticed your comment here a few months after you wrote it. :)

I agree. Perhaps kou can think this through a bit via Fiddle. Right now it requires too much C-specific knowledge. Perhaps we can get wrappers for subclassing and not having to assign to values merely to prevent segfaults. But I know way too little to even understand what is going on - I should have started learning C properly ... was harder after knowing ruby because ruby is more fun than C!