ashbb / green_shoes

Green Shoes is one of the colorful Shoes written in pure Ruby.
Other
204 stars 37 forks source link

make alert() thread safe. #43

Closed gutenye closed 13 years ago

gutenye commented 13 years ago

Gtk::Dialog#run() is not thread safe.

for Example

Shoes.app do
  Thread.start do
     dialog = Gtk::Dialog.new
     dialog.run
  end
end

the main window will be freezed.

more info at http://stackoverflow.com/questions/3504739/twisted-gtk-should-i-run-gui-things-in-threads-or-in-the-reactor-thread

ashbb commented 13 years ago

Hi Guten,

Thank you for the interesting patch. Looks good. :)

But before merge your pull request, I have two questions.

First, after added your patch on my working directory for Green Shoes, but the example snippet you showed on the first post was still freezed.

Second, after added your patch, the following snippet doesn't block to show the Shoes main window until you click an 'ok' button on the alert window. In Red Shoes, the alert window blocks the main window.

require 'green_shoes'
Shoes.app do
  alert 'hello'
end

ashbb

gutenye commented 13 years ago

For your first question, let's change to another example.

Shoes.app do
  Thread.start do
     sleep 1
     alert "hello"
  end
   button("foo"){p 1}
end

Before my patch, It doesn't work, it's freezed.

After my patch, it works. It builds the GUI with a foo button, and after 1 second, alert("hello").

For your second question. That's really a problem. Maybe be we can keep the original alert() method unchanged., but add another method called alert_unblock() which used my patch. further, the alert_unblock() method can be integrated into original alert() method with an option, like alert("hello", :block => false)

ashbb commented 13 years ago

Hi Guten,

alert("hello", :block => false) is a good idea. I like it. :)

But on my Windows 7, after your patch, it doesn't work. :( Yes, it builds the GUI with a foo button, but after 1 second, it crashes...

Umm,... what platform are you using?

ashbb

gutenye commented 13 years ago

Ruby1.9.2, ArchLinux.

work well, It's maybe a win32 problem.

Guten

ashbb commented 13 years ago

Yeah, I think so, too. Okay, could you sent me a pull request for implement alert("hello", :block => false)? I'll merge. :)

gutenye commented 13 years ago

yes, updated the code.

ashbb commented 13 years ago

Thanks. But Looks like alert 'hello', block: false and alert 'hello' are same behavior...

How about the following? ;-)

def alert msg, options={block: true}
  #options[:block] ||= true

If it's okay, please send a pull request again.

gutenye commented 13 years ago

updated the code. It's okay now. I've tested. thanks for the code.

ashbb commented 13 years ago

Merged the pull request and deleted a few debug lines and updated master branch. This is the first commit after v1.0. Thanks!