Intermittent segfault when freeing MMManager object using g_object_unref()

[Andrew Stoehr]

Hello again,

> https://aleksander.es/data/valgrind-memcheck.pdf :)

Wow, this is a great resource!  Thanks!

> How are you running the valgrind test? Are you running it with
> G_SLICE=always-malloc?
> What version of GLib are you using? I hope it's GLib >= 2.55.1.

I was not including G_SLICE=always-malloc before, but I am using GLib 2.64.4.

> But that would not circumvent any leak, you would be moving it to a
> different place. Or do you mean you would fork a process every time
> you need to run some libmm-glib operation?

Yes, that is what I was originally thinking -- forking a new process every time.  Definitely not ideal, I know :)

> What's happening here is that there is some g_object_unref() inside
> GIO that *creates* an idle source and attaches it to the main context
> in GLib. The purpose, I assume, is to run something once the object
> has been fully disposed, but in your case, the problem is that as
> there is no main loop, those idles keep being added for every unref
> you run.

This makes a lot of sense.  I really appreciate the explanation.  I was curious before how things got cleaned up, and it seems obvious now that something needs to be running in the background taking care of that.

> You should be able to solve this issue without further help or change
> in libmm-glib or glib by switching your logic to use a main loop and
> use the async APIs instead or in addition to the sync ones. This is
> relatively easy to do, even if your program doesn't use a GLib
> mainloop, because you could have all libmm-glib related stuff running
> in a separate thread with its own mainloop inside the thread, and then
> syncing whatever info you need via mutexes or something like that.

I was able to incorporate this by doing:

    GMainLoop *g_loop = NULL;
    GThread *g_loop_thread = NULL;

    g_loop = g_main_loop_new (NULL, FALSE);
    g_loop_thread = g_thread_new(NULL, (GThreadFunc)g_main_loop_run, g_loop);

And then cleaning up the main loop and thread after my application's loop.  The application loop is unchanged otherwise, still using the "sync" APIs.  Seems to work really well -- thanks for the guidance.

> If you cannot change the whole logic to use a main loop in a separate
> thread, you could still have a dummy main loop running some iterations
> for some time; e.g. in your tester, instead of the usleep() call you
> could put:
> 
>    {
>         GMainLoop *loop;
> 
>         loop = g_main_loop_new (NULL, FALSE);
>         g_idle_add ((GSourceFunc)stop_loop, loop);
>         g_main_loop_run (loop);
>         g_main_loop_unref (loop);
>     }
> 
> with a separate stop_loop method like this:
> 
> static gboolean
> stop_loop (GMainLoop *loop)
> {
>     g_main_loop_quit (loop);
>     return G_SOURCE_REMOVE;
> }
> 
> What we're doing is creating a main loop that will run the events in
> the main context (the GDBus setup would have scheduled events in that
> main context). And we also add our own idle source in the main
> context, and given that idles are all scheduled one after the other,
> our idle source would be scheduled LAST. And then, in the idle source
> we stop the main loop, as there were no more idles scheduled
> afterwards. It's a bit hacky, but should work when you need to
> synchronize the idles setup by GDBus. Note that the main loop will run
> exclusively for the time needed to cleanup the event list until your
> idle, so that should be extremely quick.

> After that change, I cannot see any major still reachable leak
> happening any more, see the attached source file with my changes.

This is a clever workaround!  I tried with this method also, just for fun, and I did not see any more leaking memory in my application.  I did notice the memory usage for dbus-daemon increased steadily over a period of time, though -- with my application running overnight, it grew to take up about 20% of my hardware's 128MB of RAM.  I did not see this increase with the main loop in a separate thread as I mentioned above, so it might have something to do with the constant creation and destruction of main loops, or perhaps I am just missing something.  I figured I would mention it to you anyways just in case it is something you want to investigate.

> I should definitely document this solution in modemmanager.org...

I agree -- it is a really convenient way to get the functionality while remaining synchronous.

Thank you so much for all of the help and information -- you are a lifesaver!

Best,

Andrew