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 767516 - XInitThreads not called in empathy-call
XInitThreads not called in empathy-call
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: VoIP
3.12.x
Other Linux
: Normal normal
: ---
Assigned To: empathy-maint
empathy-maint
Depends on:
Blocks:
 
 
Reported: 2016-06-10 21:05 UTC by Diane Trout
Modified: 2016-07-22 14:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
stack trace of the segfault (87.40 KB, text/plain)
2016-06-10 21:05 UTC, Diane Trout
  Details
proposed fix for wayland guard (986 bytes, patch)
2016-06-10 21:07 UTC, Diane Trout
needs-work Details | Review
Alwyas initialize X library (1.04 KB, patch)
2016-06-24 05:36 UTC, Diane Trout
none Details | Review
Revert "Don't call XInitThreads in Wayland" (1.45 KB, patch)
2016-07-22 01:50 UTC, Michael Catanzaro
committed Details | Review

Description Diane Trout 2016-06-10 21:05:24 UTC
Created attachment 329590 [details]
stack trace of the segfault

I discovered when trying to test empathy-call that it segfaulted in __GI____pthread_mutex_lock nptl/pthread_mutex_lock.c 

I later discovered that XInitThreads wasn't being called because the guard condition to prevent calling it under wayland was always false. 

in empathy-call.c:main

  if (GDK_IS_X11_DISPLAY (gdk_display_get_default ())) 

is false until after g_object_context_parse is called. Unfortunately calling XInitThreads after that doesn't prevent the segfault.

I replaced it with this check for wayland which did work under X. Though I haven't tested it under wayland yet. (I did the get the idea from a WIP patch for porting firefox to wayland.)

  if (!g_getenv("WAYLAND_DISPLAY")) {

(I'm running on debian unstable, empathy-call built from git, using commit cb4d5c2b8023c2f2a9df1b8a9fb19f2af7d392be)
Comment 1 Diane Trout 2016-06-10 21:07:54 UTC
Created attachment 329591 [details] [review]
proposed fix for wayland guard
Comment 2 Diane Trout 2016-06-11 04:25:15 UTC
Just tested this patch under GNOME wayland and was able to make a video call.

Shockingly it seemed like all 4 streams actually connected - I had video in both directions and and audio.

I only had video in one direction under X.

I suspect I was just lucky.

Regardless, there were no thread related crashes, so I think this should go in.
Comment 3 Matthias Clasen 2016-06-13 19:22:34 UTC
I don't think that is right. We don't want to depend on the WAYLAND_DISPLAY environment variable. The wayland socket is in a well-known location, and we will connect to it regardless whether WAYLAND_DISPLAY is set or not.
Comment 4 Karl Tomlinson 2016-06-13 20:13:07 UTC
XInitThreads() initializes the library rather than a Display connection, and so it should be fine to call regardless of the display type.
Comment 5 Debarshi Ray 2016-06-15 09:43:09 UTC
Review of attachment 329591 [details] [review]:

Setting the patch status as per comment 3 and comment 4
Comment 6 Diane Trout 2016-06-24 05:36:27 UTC
Created attachment 330288 [details] [review]
Alwyas initialize X library

I finally found some time to test Karl Tomlinson's suggestion and yes it appears to be safe to always call XInitThreads.

Tried under GNOME Wayland and GNOME under X, empathy-call ran without immediately segfaulting, and sometimes would manage to negotiate video.
Comment 7 Michael Catanzaro 2016-07-21 22:35:28 UTC
Thanks for investigating Diane and sorry about this. Let's just revert my change that broke it for no reason.
Comment 8 Michael Catanzaro 2016-07-22 01:50:06 UTC
The following fix has been pushed:
bd81961 Revert "Don't call XInitThreads in Wayland"
Comment 9 Michael Catanzaro 2016-07-22 01:50:09 UTC
Created attachment 331950 [details] [review]
Revert "Don't call XInitThreads in Wayland"

This reverts commit a9ede294c57bc5738be3c33cba4ef88b9a7d4a0e.

It causes empathy-call to crash when run in X11, and fixes absolutely
nothing. See the bug for details.

Thanks to Diane Trout for investigating and preparing a substantially-
identical patch.