After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 736941 - computer-player: don't run search on UI thread
computer-player: don't run search on UI thread
Status: RESOLVED FIXED
Product: iagno
Classification: Applications
Component: general
git master
Other All
: Normal normal
: ---
Assigned To: iagno-maint
iagno-maint
Depends on: 736932
Blocks:
 
 
Reported: 2014-09-19 04:50 UTC by Michael Catanzaro
Modified: 2014-09-24 13:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
computer-player: don't run search on UI thread (1.02 KB, patch)
2014-09-19 04:50 UTC, Michael Catanzaro
committed Details | Review
Require vala 0.25.1 (628 bytes, patch)
2014-09-20 16:16 UTC, Michael Catanzaro
committed Details | Review
computer-player: only use GTK+ on the UI thread (882 bytes, patch)
2014-09-20 16:16 UTC, Michael Catanzaro
committed Details | Review
computer-player: fix cancel_move() (3.86 KB, patch)
2014-09-21 20:57 UTC, Michael Catanzaro
none Details | Review
computer-player: fix cancel_move() (4.33 KB, patch)
2014-09-23 14:23 UTC, Michael Catanzaro
committed Details | Review
Fix cancel_move(). (4.46 KB, patch)
2014-09-24 07:03 UTC, Arnaud B.
none Details | Review

Description Michael Catanzaro 2014-09-19 04:50:42 UTC
Don't run the search on the UI thread.
Comment 1 Michael Catanzaro 2014-09-19 04:50:43 UTC
Created attachment 286563 [details] [review]
computer-player: don't run search on UI thread

It doesn't matter at the low depths we use, but it'd be better not to
block the UI for the duration of the search.
Comment 2 Michael Catanzaro 2014-09-19 13:56:58 UTC
Attachment 286563 [details] pushed as 8bcb83c - computer-player: don't run search on UI thread
Comment 3 Arnaud B. 2014-09-19 16:16:00 UTC
I reopen (if it works this time xD ) as I have many bugs since this morning.
I had some:

(iagno:30047): Gdk-WARNING **: iagno: Fatal IO error 11 (Ressource temporairement non disponible) on X server :0.

but I’m not sure they are related.
But now, I have a:

[xcb] Unknown sequence number while processing queue
[xcb] Most likely this is a multi-threaded client and XInitThreads has not been called
[xcb] Aborting, sorry about that.
iagno: xcb_io.c :274 : poll_for_event:  l'assertion « !xcb_xlib_threads_sequence_lost » a échoué.
Abandon (core dumped)

and I’m quite sure it is. The command-line was `jhbuild run iagno -l 3 --fast-mode` with all my currently published patches applied.
Comment 4 Michael Catanzaro 2014-09-19 19:48:47 UTC
I got the first error too, and assumed I was leaking threads, but I'm sure that's not happening, so I'm not sure what's wrong. And that second error looks like gobbledygook to me.

Anyway I'm pretty sure my patch is indeed causing these.
Comment 5 Michael Catanzaro 2014-09-19 20:54:02 UTC
How easy is it for you to reproduce this? I got your first error once, but have had no luck since then.

From the error message, my guess is that we're accidentally using a GTK+ function from the AI thread, which is unsafe. But I don't see where or how we could be doing that.
Comment 6 Michael Catanzaro 2014-09-19 20:54:55 UTC
(In reply to comment #5)
> How easy is it for you to reproduce this?

Meaning, if you could get a backtrace in gdb ('thread apply all bt full'), that would be great....
Comment 7 Michael Catanzaro 2014-09-20 15:28:55 UTC
Got a backtrace with G_DEBUG=fatal-warnings


Thread 9 (Thread 0x7fffd3c90700 (LWP 20521))

  • #0 g_logv
    at gmessages.c line 1046
  • #1 g_log
  • #2 gdk_x_io_error
    at gdkmain-x11.c line 252
  • #3 _XIOError
    at XlibInt.c line 1498
  • #4 _XReply
    at xcb_io.c line 708
  • #5 XGetGeometry
    at GetGeom.c line 47
  • #6 gdk_window_x11_get_geometry
    at gdkwindow-x11.c line 3066
  • #7 gdk_window_get_geometry
    at gdkwindow.c line 5950
  • #8 gdk_screen_get_monitor_at_window
    at gdkscreen.c line 308
  • #9 ca_gtk_proplist_set_for_widget
    at canberra-gtk.c line 285
  • #10 ca_gtk_play_for_widget
    at canberra-gtk.c line 445
  • #11 iagno_play_sound
    at iagno.c line 1238
  • #12 iagno_game_move_cb
    at iagno.c line 1075
  • #13 _g_closure_invoke_va
    at gclosure.c line 831
  • #14 g_signal_emit_valist
    at gsignal.c line 3218
  • #15 g_signal_emit_by_name
    at gsignal.c line 3405
  • #16 game_place_tile
    at game.c line 1113
  • #17 computer_player_complete_move
    at computer-player.c line 240
  • #18 computer_player_move_async_co
    at computer-player.c line 402
  • #19 __lambda4_
    at computer-player.c line 319
  • #20 ___lambda4__gthread_func
    at computer-player.c line 327
  • #21 g_thread_proxy
    at gthread.c line 764
  • #22 start_thread
    at pthread_create.c line 309
  • #23 clone
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S line 111

Comment 8 Michael Catanzaro 2014-09-20 16:16:17 UTC
Created attachment 286690 [details] [review]
Require vala 0.25.1

For Source.REMOVE
Comment 9 Michael Catanzaro 2014-09-20 16:16:20 UTC
Created attachment 286691 [details] [review]
computer-player: only use GTK+ on the UI thread
Comment 10 Michael Catanzaro 2014-09-20 16:33:30 UTC
Attachment 286690 [details] pushed as 6899edc - Require vala 0.25.1
Attachment 286691 [details] pushed as 2fe4898 - computer-player: only use GTK+ on the UI thread
Comment 11 Arnaud B. 2014-09-21 12:11:15 UTC
I reopen. There’s a crash when you undo one of your moves and the AI already began to think (like it’s usual in level 3, until my next patch is pushed), after the AI thread finished its calculations.
Comment 12 Michael Catanzaro 2014-09-21 20:57:58 UTC
Created attachment 286758 [details] [review]
computer-player: fix cancel_move()
Comment 13 Arnaud B. 2014-09-22 06:21:14 UTC
The game is blocked if you undo fast two times and play, it’s the opponent turn and it doesn’t think. ^^
Comment 14 Michael Catanzaro 2014-09-23 14:23:31 UTC
Created attachment 286878 [details] [review]
computer-player: fix cancel_move()
Comment 15 Arnaud B. 2014-09-24 07:03:32 UTC
Created attachment 286946 [details] [review]
Fix cancel_move().

Looks like it works, even if I don’t understand all. Here is an updated patch that works on top of the “Faster history” patches, if needed.
Comment 16 Michael Catanzaro 2014-09-24 13:37:43 UTC
Each yield statement causes the function to return, and each call to move_async.callback() causes it to resume from where it previously yielded.

I feel like the code is more complicated than it ought to be, though. :/

Thanks for the free rebase.
Comment 17 Michael Catanzaro 2014-09-24 13:40:22 UTC
Actually, for some reason my patch applies without change, and yours doesn't at all. Are you sure you haven't screwed up your local branches again? :)  Whatever....
Comment 18 Michael Catanzaro 2014-09-24 13:40:41 UTC
Comment on attachment 286878 [details] [review]
computer-player: fix cancel_move()

Attachment 286878 [details] pushed as 0f7e313 - computer-player: fix cancel_move()