Bug 44447 - Gabble plugin crashes on windows
Summary: Gabble plugin crashes on windows
Alias: None
Product: Ytstenut
Classification: Unclassified
Component: plugins (show other bugs)
Version: unspecified
Hardware: Other Windows (All)
: medium normal
Assignee: Siraj Razick
QA Contact:
URL: http://cgit.collabora.com/git/user/si...
Whiteboard: review+
Keywords: patch
Depends on: 44331 44649 46245
  Show dependency treegraph
Reported: 2012-01-04 02:45 UTC by Siraj Razick
Modified: 2012-02-17 13:14 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Description Siraj Razick 2012-01-04 02:45:29 UTC
The gabble plugin for ytstenut crashes on windows when we run the server-nosey-status.
Comment 1 Olli Salli 2012-01-04 14:43:42 UTC
It crashes because the current plugin architecture of Gabble has some constraints which can't be fulfilled on Windows... stay tuned. More details in bug 44331, comment 6 onwards.

Siraj, after you've been able to make a gabble build disabling the asserts checking for those constraints, could you file logs and backtraces for any further crashes you encounter here, please?

Anyway, properly fixing this will likely require changes to both Gabble's plugin architecture and the ytstenut plugin (And other plugins, which still want to stay compatible with newer gabble versions, if the changes are backwards incompatible as it looks like they must be!). So adding dep.
Comment 2 Olli Salli 2012-01-12 01:43:45 UTC
Siraj, please add a branch which ports the Ytstenut plugin to the new windows-feasible Gabble plugin API when that gets finalized. We'll review it here as per Ytstenut review procedure.
Comment 3 Olli Salli 2012-01-23 10:37:36 UTC
Siraj's patch is at URL.

It basically does
 * change all references to GabbleConnection to the GabblePluginConnection interface, as needed per bug 44649 changes (straightforward)
 * change the source tag used for create_sidecar to the symbol from the plugin, and rename the function to have a _async suffix, as per bug 44331 changes


in ytstenut_plugin_create_sidecar_async:

      /* sic: all plugins share {salut,gabble}_plugin_create_sidecar_finish() so we
       * need to use the same source tag.

As of this patch, this is no longer true for gabble... so please move to salut section, and only mention salut in this comment, so you remember to take it out completely when the similar change is done for the salut part.

+  g_simple_async_result_is_valid ((GAsyncResult*) result, G_OBJECT (plugin), NULL);

Calling _is_valid and ignoring the return value doesn't make sense. This function is usually used to check/assert that a GSimpleAsyncResult received in a callback is what it should be; here, however you've just created it, and set it finished, thus you already know it's valid. So what are you actually trying to do?

--- a/plugin-base/ytstenut.c
+++ b/plugin-base/ytstenut.c
@@ -19,6 +19,7 @@
 #include "config.h"
+#include <stdio.h>


-  iface->create_sidecar = ytstenut_plugin_create_sidecar;
+  iface->create_sidecar_async = ytstenut_plugin_create_sidecar_async;
+  iface->create_sidecar_finish = ytstenut_plugin_create_sidecar_finish;

This will break the Salut build until you've made similar changes to Salut plugin API as you now did for Gabble. So, please #ifdef this, so that the old method in vtable is still used for Salut.
Comment 4 Olli Salli 2012-01-23 10:38:45 UTC
URL actually pointed to outdated patch, now correct one for the branch
Comment 5 Siraj Razick 2012-01-24 17:46:24 UTC
branch updated with review comments and other fixes.
Comment 6 Siraj Razick 2012-01-24 17:56:05 UTC
once all the patches are merged.. this patch will produce .dll files on windows
Comment 7 Siraj Razick 2012-01-24 18:02:13 UTC
gush sorry wrong bug
Comment 8 Olli Salli 2012-01-26 03:26:56 UTC
Looks like all the issues are fixed. Thanks!
Comment 9 Olli Salli 2012-02-08 11:30:41 UTC
Has been merged a long time ago...

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.