Comment 13 for bug 738584

Revision history for this message
In , Mathieu Trudel-Lapierre (cyphermox) wrote :

Created attachment 45389
don't assert for position/address to be NULL when initializing interfaces

I'm not certain of the exact case since I haven't been able to reproduce the crash at all, no matter how hard I tried, but I think the following would make sense:

@@ -819,20 +819,20 @@
        }

        if (priv->interfaces & GC_IFACE_POSITION) {
- g_assert (priv->position == NULL);
-
- priv->position = geoclue_position_new (priv->service,
- priv->path);
- g_signal_connect (G_OBJECT (priv->position), "position-changed",
- G_CALLBACK (position_changed), provider);
+ if (priv->position == NULL) {
+ priv->position = geoclue_position_new (priv->service,
+ priv->path);
+ g_signal_connect (G_OBJECT (priv->position), "position-changed",
+ G_CALLBACK (position_changed), provider);
+ }
        }
        if (priv->interfaces & GC_IFACE_ADDRESS) {

If for some reason the position object isn't NULL (e.g. which I guess could be possible on resume, see below), merrily carry on.

I think what's happening is in the case of this bug report, the system resumes and NM quickly cycles through the various states, probably fast enough to online that the master provider gets to call:

        /* update connection-cacheable providers */
        if (status == GEOCLUE_CONNECTIVITY_ONLINE &&
            priv->provides & GEOCLUE_PROVIDE_CACHEABLE_ON_CONNECTION) {
                /* intialize to fill cache (this will handle status change) */
                if (gc_master_provider_initialize (provider)) {
                        gc_master_provider_deinitialize (provider);
                }

Tries to initialize the interfaces, and fails because position or address are already filled.

Attached is the "full" patch for the piece of code I added above. Might be cleaner to deinit it and fill it back though.