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 671931 - Crash receiving a call on a already disconnected empathy call window
Crash receiving a call on a already disconnected empathy call window
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: VoIP
3.3.x
Other Linux
: Normal critical
: ---
Assigned To: Emanuele Aina
empathy-maint
: 671354 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2012-03-12 18:19 UTC by Emanuele Aina
Modified: 2012-03-16 10:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Log from gabble (48.69 KB, text/plain)
2012-03-12 18:22 UTC, Emanuele Aina
  Details
Log from empathy-call (9.73 KB, text/plain)
2012-03-12 18:26 UTC, Emanuele Aina
  Details
Disconnect when a call is ENDED (1.40 KB, patch)
2012-03-13 16:21 UTC, Emanuele Aina
reviewed Details | Review
Don't restart a call if one already in progress (1.20 KB, patch)
2012-03-13 16:21 UTC, Emanuele Aina
accepted-commit_now Details | Review
Disconnect also on the insufficient balance case. (1.26 KB, patch)
2012-03-14 09:01 UTC, Emanuele Aina
reviewed Details | Review
Removed redundant state check. (1.22 KB, patch)
2012-03-14 09:12 UTC, Emanuele Aina
reviewed Details | Review
Create outgoing call windows in RINGING state (3.77 KB, patch)
2012-03-14 21:47 UTC, Emanuele Aina
reviewed Details | Review
Don't restart a call if one already in progress (1.20 KB, patch)
2012-03-14 21:47 UTC, Emanuele Aina
reviewed Details | Review
Re-spin, disconnect when a call is ENDED, removing a call to (1.45 KB, patch)
2012-03-14 21:53 UTC, Emanuele Aina
reviewed Details | Review
Added a commend about relying on the ENDED call state signal to (1.58 KB, patch)
2012-03-15 12:25 UTC, Emanuele Aina
committed Details | Review
Do not restart calls in RINGING state. (1.24 KB, patch)
2012-03-15 12:29 UTC, Emanuele Aina
reviewed Details | Review
Don't restart a call if one already in progress (1.33 KB, patch)
2012-03-15 17:29 UTC, Emanuele Aina
committed Details | Review

Description Emanuele Aina 2012-03-12 18:19:20 UTC


  • #0 raise
    from /lib/x86_64-linux-gnu/libc.so.6
  • #1 abort
    from /lib/x86_64-linux-gnu/libc.so.6
  • #2 g_assertion_message
  • #3 g_assertion_message_expr
    at /tmp/buildd/glib2.0-2.30.2/./glib/gtestutils.c line 1436
  • #4 create_video_output_widget
    at empathy-call-window.c line 395
  • #5 empathy_call_window_restart_call
    at empathy-call-window.c line 3999
  • #6 ffi_call_unix64
    from /usr/lib/x86_64-linux-gnu/libffi.so.5
  • #7 ffi_call
    from /usr/lib/x86_64-linux-gnu/libffi.so.5
  • #8 g_cclosure_marshal_generic
    at /tmp/buildd/glib2.0-2.30.2/./gobject/gclosure.c line 1147
  • #9 g_closure_invoke
    at /tmp/buildd/glib2.0-2.30.2/./gobject/gclosure.c line 774
  • #10 signal_emit_unlocked_R
    at /tmp/buildd/glib2.0-2.30.2/./gobject/gsignal.c line 3272
  • #11 g_signal_emit_valist
    at /tmp/buildd/glib2.0-2.30.2/./gobject/gsignal.c line 3003
  • #12 g_signal_emit
    at /tmp/buildd/glib2.0-2.30.2/./gobject/gsignal.c line 3060
  • #13 handle_channels
    at empathy-call-factory.c line 228
  • #14 handle_channels_context_prepare_cb
    at base-client.c line 2227
  • #15 g_simple_async_result_complete
    at /tmp/buildd/glib2.0-2.30.2/./gio/gsimpleasyncresult.c line 749
  • #16 context_check_prepare
    at handle-channels-context.c line 556
  • #17 context_check_prepare
    at handle-channels-context.c line 741
  • #18 hcc_channel_prepare_cb
    at handle-channels-context.c line 629
  • #19 g_simple_async_result_complete
    at /tmp/buildd/glib2.0-2.30.2/./gio/gsimpleasyncresult.c line 749
  • #20 complete_in_idle_cb
    at /tmp/buildd/glib2.0-2.30.2/./gio/gsimpleasyncresult.c line 761
  • #21 g_main_dispatch
    at /tmp/buildd/glib2.0-2.30.2/./glib/gmain.c line 2442
  • #22 g_main_context_dispatch
    at /tmp/buildd/glib2.0-2.30.2/./glib/gmain.c line 2998
  • #23 g_main_context_iterate
    at /tmp/buildd/glib2.0-2.30.2/./glib/gmain.c line 3076
  • #24 g_main_loop_run
    at /tmp/buildd/glib2.0-2.30.2/./glib/gmain.c line 3284
  • #25 gtk_main
    at /tmp/buildd/gtk+3.0-3.2.3/./gtk/gtkmain.c line 1362
  • #26 g_application_run
    at /tmp/buildd/glib2.0-2.30.2/./gio/gapplication.c line 1323
  • #27 main
    at empathy-call.c line 258

Comment 1 Emanuele Aina 2012-03-12 18:22:40 UTC
Created attachment 209519 [details]
Log from gabble
Comment 2 Emanuele Aina 2012-03-12 18:26:15 UTC
Created attachment 209520 [details]
Log from empathy-call
Comment 3 Emanuele Aina 2012-03-13 11:20:37 UTC
I learned how to replicate it while doing some test for bug #671722.
- A@gmail.com is set as invisible and starts a call from the web ui for B@gmail.com, who uses empathy
- B@gmail.com accept the calls
- in the web ui A@gmail.com sees the call as dropped per bug #671722, while empathy still waits with the "hangup" button shown
- A@gmail.com starts a new call
- B@gmail.com accept the calls
- empathy crashes as there already is a video element instantiated and the assertion in create_video_output_widget() fails
Comment 4 Guillaume Desmottes 2012-03-13 11:38:16 UTC
*** Bug 671354 has been marked as a duplicate of this bug. ***
Comment 5 Emanuele Aina 2012-03-13 16:21:49 UTC
Created attachment 209618 [details] [review]
Disconnect when a call is ENDED

If for some unspecified reason a call go to the TP_CALL_STATE_ENDED
state, reinitialize the call window.
Comment 6 Emanuele Aina 2012-03-13 16:21:57 UTC
Created attachment 209619 [details] [review]
Don't restart a call if one already in progress

Fixes the crash when calling someone while being already in a call
with the same contact.
Comment 7 Guillaume Desmottes 2012-03-13 16:40:07 UTC
Review of attachment 209618 [details] [review]:

::: src/empathy-call-window.c
@@ +3193,3 @@
+      else {
+          DEBUG ("Call ended: %s", (reason != NULL && reason[0] != '\0') ? reason : "unspecified reason");
+          empathy_call_window_disconnected (self, TRUE);

Shouldn't we do this in the balance case as well? It's not that different, we just display a specific error for it.
Comment 8 Guillaume Desmottes 2012-03-13 16:40:40 UTC
Review of attachment 209619 [details] [review]:

++
Comment 9 Emanuele Aina 2012-03-14 09:01:56 UTC
Created attachment 209680 [details] [review]
Disconnect also on the insufficient balance case.
Comment 10 Guillaume Desmottes 2012-03-14 09:07:48 UTC
Review of attachment 209680 [details] [review]:

::: src/empathy-call-window.c
@@ +3193,3 @@
+      DEBUG ("Call ended: %s", (reason != NULL && reason[0] != '\0') ? reason : "unspecified reason");
+      empathy_call_window_disconnected (self, TRUE);
+      if (state == TP_CALL_STATE_ENDED &&

no need to re-check the state twice
Comment 11 Emanuele Aina 2012-03-14 09:12:15 UTC
Created attachment 209682 [details] [review]
Removed redundant state check.
Comment 12 Guillaume Desmottes 2012-03-14 09:18:18 UTC
Review of attachment 209682 [details] [review]:

empathy_call_window_disconnected() is called in a few other place. Did you check if this patch wasn't introducing redundant calls (and so redundant work) when the channel was closed or when manually hanging up?
Comment 13 Emanuele Aina 2012-03-14 21:47:20 UTC
Created attachment 209783 [details] [review]
Create outgoing call windows in RINGING state

This commit is needed for 'Don't restart a call if one already 
in progress', otherwise outgoing calls will not have the call
handler attached.
Comment 14 Emanuele Aina 2012-03-14 21:47:43 UTC
Created attachment 209784 [details] [review]
Don't restart a call if one already in progress

Fixes the crash when calling someone while being already in a call
with the same contact.
Comment 15 Emanuele Aina 2012-03-14 21:53:45 UTC
Created attachment 209786 [details] [review]
Re-spin, disconnect when a call is ENDED, removing a call to

empathy_call_window_disconnected().

I can't say if the other calls are still needed, likely they may be
replaced by something making the call go to the ENDED state.

Also, all calls to empathy_call_window_disconnected() have the
'restart' parameter set to TRUE, it may make sense to remove it
altogether.
Comment 16 Guillaume Desmottes 2012-03-15 08:26:02 UTC
Review of attachment 209783 [details] [review]:

Hum either I don't understand this patch or the commit message is wrong. Shouldn't it be "Create *incoming* call windows in RINGING state" ?

Also, so rational about this change in the long commit message would be welcome.
Comment 17 Guillaume Desmottes 2012-03-15 08:28:11 UTC
Review of attachment 209784 [details] [review]:

::: src/empathy-call-window.c
@@ +2441,3 @@
+
+  if (self->priv->call_state == CONNECTING ||
+      self->priv->call_state == CONNECTED)

What about the ringing state? Do we we really want to start a new call in this case?
Comment 18 Guillaume Desmottes 2012-03-15 08:35:09 UTC
Review of attachment 209786 [details] [review]:

::: src/empathy-call-window.c
@@ -4008,3 @@
   empathy_call_handler_stop_call (self->priv->handler);
-
-  empathy_call_window_disconnected (self, TRUE);

Just to be sure; we now rely on stop_call() to change the state of ENDED to call window_disconnected() right? May be worth a comment as this code is getting pretty hairy.
Comment 19 Emanuele Aina 2012-03-15 09:13:30 UTC
Review of attachment 209783 [details] [review]:

Oops, of course you're right, I wrote the wrong commit message.
Comment 20 Emanuele Aina 2012-03-15 12:25:49 UTC
Created attachment 209820 [details] [review]
Added a commend about relying on the ENDED call state signal to

clean up on hangup.
Comment 21 Emanuele Aina 2012-03-15 12:29:23 UTC
Created attachment 209821 [details] [review]
Do not restart calls in RINGING state.

Also, the outgoing call windows in RINGING state patch is no longer
strictly needed, so I've withdrawn it but it may still make sense on
its own in a separate bug.
Comment 22 Guillaume Desmottes 2012-03-15 13:13:24 UTC
Review of attachment 209820 [details] [review]:

++
Comment 23 Guillaume Desmottes 2012-03-15 14:12:05 UTC
Review of attachment 209821 [details] [review]:

::: src/empathy-call-window.c
@@ +2432,3 @@
+
+  if (self->priv->call_state == CONNECTING ||
+      self->priv->call_state == CONNECTED ||

Actually don't we just want to do this only if state is DISCONNECTED?
Comment 24 Guillaume Desmottes 2012-03-15 15:17:14 UTC
Comment on attachment 209820 [details] [review]
Added a commend about relying on the ENDED call state signal to

I already merged the first patch.
Comment 25 Emanuele Aina 2012-03-15 17:29:17 UTC
Created attachment 209867 [details] [review]
Don't restart a call if one already in progress

Re-spin checking only for the DISCONNECTED state
Comment 26 Guillaume Desmottes 2012-03-16 10:21:32 UTC
Attachment 209867 [details] pushed as 1e03889 - Don't restart a call if one already in progress