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 139699 - Correction for g_main_context_unref()
Correction for g_main_context_unref()
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: mainloop
2.4.x
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2004-04-11 07:59 UTC by Philippe Blain
Modified: 2014-01-19 23:03 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
GMainContext: unref pending sources on destroy (3.15 KB, patch)
2013-10-25 05:49 UTC, Allison Karlitskaya (desrt)
committed Details | Review

Description Philippe Blain 2004-04-11 07:59:49 UTC
File: glib-2.4.0/glib/gmain.c
Function: g_main_context_unref()

When sources are added to the pending list, they are ref(). (line 2408)
But, when the pending list is destroyed, sources MUST be unref().
This has been forgotten in 'g_main_context_unref()'.
It should look like :

void g_main_context_unref (GMainContext * context)
{
    ..................................
    G_LOCK (main_context_list);
    main_context_list = g_slist_remove (main_context_list, context);
    G_UNLOCK (main_context_list);

    /* Destroy pending sources list */
==> for (i = 0; i < context->pending_dispatches->len; i++) {
==>     if (context->pending_dispatches->pdata[i])
==>         SOURCE_UNREF ((GSource *) context->pending_dispatches->pdata[i],
==>                       context);
==> }
    g_ptr_array_free (context->pending_dispatches, TRUE);

    source = context->source_list;
    while (source) {
        GSource *next = source->next;
        g_source_destroy_internal (source, context, TRUE);
        source = next;
    }
    ..................................
}
Comment 1 Matthias Clasen 2004-04-23 15:13:11 UTC
I'm not sure if this is needed. If you look up a bit you'll see that all sources
from context->source_list are destroyed. It seems to me that the
pending_dispatches are always from that list, so they'll be gone anyway at that
point...

Comment 2 Philippe Blain 2004-04-26 18:03:39 UTC
My point of view:

1) A source attached to a context is ref().
   (line 929 + g_source_list_add() )

2) That source, when added to the pending list, is also ref().
   (line 2408 + g_ptr_array_add() )

Now, when calling g_main_context_unref(); you break type 1) links.
Sources are marked as destroyed, and unref() by g_source_destroy_internal():

    source = context->source_list;
    while (source) {
        GSource *next = source->next;
        g_source_destroy_internal (source, context, TRUE);
        source = next;
    }
    ..................................

But, before freeing context and the array of pending_dispatches by
    g_ptr_array_free (context->pending_dispatches, TRUE);
the type 2) links must be break by unreferencing the sources again.

Missing is the same piece of code as in g_main_context_prepare();
or in g_main_dispatch() where sources are unref() before
    g_ptr_array_set_size (context->pending_dispatches, 0);
Comment 3 Matthias Clasen 2004-11-21 06:16:09 UTC
The analysis looks right to me. 

But I have a hard time coming up with a non-broken example where
g_main_context_unref () could be called with pending_dispatches being
nonempty. Here is an example which does in fact manage to loose reference
to a source in the way described by Philippe, but it doesn't matter much
since we segfault a millisecond later anyway.



#include <glib.h>

static gboolean 
foo1 (gpointer data)
{
  GMainContext *context = (GMainContext *)data;
  
  g_main_context_unref (context);

  return TRUE;
}

static gboolean 
foo2 (gpointer data)
{
  return TRUE;
}


int
main (int argc, char *argv[])
{
  GMainContext *context;
  GMainLoop *loop;

  context = g_main_context_default ();
  g_idle_add (foo1, context);
  g_idle_add (foo2, context);
  
  loop = g_main_loop_new (NULL, FALSE);

  g_main_loop_run (loop);
  return 0;
}
Comment 4 Allison Karlitskaya (desrt) 2013-10-25 05:49:19 UTC
Created attachment 258077 [details] [review]
GMainContext: unref pending sources on destroy

It is possible (but unlikely) that there will be a non-empty list of
pending dispatches when we remove the last ref from a GMainContext.
Make sure we drop the refs on the sources appropriately.

Add a (now-working) testcase that demonstrates how to trigger the issue.
Comment 5 Matthias Clasen 2014-01-19 23:03:06 UTC
Attachment 258077 [details] pushed as 8f6be40 - GMainContext: unref pending sources on destroy