Hi Aleksander,
Thank you for the quick reply!
> "modems" here is the list returned by the manager with
> g_dbus_object_manager_get_objects(). The list contains full
> references, so g_list_free_full() is fine.
>
> "object" is an element of that list. Are you really doing a
> g_object_ref() of the object when you're iterating? Or are you
> unref-ing the "object" without having acquired a full reference?
> Remember that you're then doing a g_list_free_full() on the whole
> object list. Maybe here's the problem?
That is a good point -- it doesn't look like I need to free "object."
> If you send the test application source code (doesn't need to be your
> full application, just a reproducer) I can give it a look; we should
> definitely confirm this is not a problem in libmm-glib itself.
I put together a small standalone application to test this out, and was able to
reproduce the segfault with the double-free you pointed out above. When I
explicitly free "object," I get the following warning message after freeing
"manager":
(process:5480): GLib-GObject-CRITICAL **: 19:45:59.258: g_object_unref:
assertion 'G_IS_OBJECT (object)' failed
When I don't explicitly free "object," I do not get that error message, and no
longer seem to get the segfault! So that issue seems to be resolved. Thanks
for catching that.
Unfortunately, it seems like there is effectively a memory leak in the code. I
have attached the standalone application to this email if you are interested in
taking a look. The two places it indicates leaks (multiple times each) are:
12,196 bytes in 239 blocks are still reachable in loss record 8 of 11
at 0x4DA4124: g_type_create_instance (in
/usr/lib/libgobject-2.0.so.0.6400.4)
23,244 bytes in 126 blocks are still reachable in loss record 9 of 11
at 0x4842B4C: realloc (in
/usr/lib/valgrind/vgpreload_memcheck-arm-linux.so)
>From running one iteration through valgrind, it looks like the "leaks"
>(valgrind reports some memory as "possibly lost" and most as "still
>reachable") are part of the library, so I do not believe I am missing any
>frees/unrefs in my code. For thoroughness, I ran mmcli through valgrind to
>print my modem information and that also reported a large amount of "possibly
>lost" and "still reachable" memory, so I believe this is expected. However,
>this memory grows each iteration, and it eventually stops working due to a
>lack of available memory. The use case for my application is to run
>continuously for longer periods of time (anywhere from hours to weeks on 128MB
>of RAM), so I don't think this is sustainable.
Is there any way I can force the library to clean up memory earlier than it
normally would? It seems like it holds onto the memory even after objects are
fully unreferenced, or perhaps the disposal functions for the objects are not
being run yet for some reason. The other thing I have been playing around with
is forking off a new process and using the library functions there to
circumvent the leak, but I would like to avoid doing it this way if possible as
it adds complexity.
Thanks for the help thus far,
Andrew
________________________________________
From: Aleksander Morgado <[email protected]>
Sent: Monday, May 31, 2021 2:47 AM
To: Andrew Stoehr
Cc: [email protected]; [email protected]
Subject: Re: Intermittent segfault when freeing MMManager object using
g_object_unref()
External email: CAUTION, be careful with links or attachments!
Hey Andrew,
> I am encountering intermittent (but easily reproducible) segfaults when
> freeing an MMManager object using g_object_unref().
>
> I am using libmm-glib from ModemManager 1.14.8 to get the state and signal
> quality on the modem I am using (u-blox TOBY-R200) on an embedded device. I
> based the usage of the API on what is done in the mmcli source code when
> printing modem information. In my code, I first create and initialize a
> GDBusConnection using g_bus_get_sync(). Then, in a 15-second loop, I:
>
> 1. make sure the GDBusConnection still exists (attempting to remake it if
> it does not)
> 2. create an MMManager for the connection with mm_manager_new_sync()
> 3. make sure ModemManager is running using
> g_dbus_object_manager_client_get_name_owner() with the manager
> 4. get a list of modems using g_dbus_object_manager_get_objects() with
> the manager
> 5. use mm_object_get_modem() on the item in the list to get the modem
> object
> 6. use mm_modem_get_state() and mm_modem_get_signal_quality() to get
> information from the modem object
> 7. free all above objects using g_object_unref() (and g_list_free_full()
> on the list)
>
> Between each step, I check to make sure there are no errors before proceeding.
>
All that seems fine.
> This process works as far as getting the data -- I am able to successfully
> read the data from the modem and it updates each loop as expected. However,
> after an indeterminate number of iterations (which varies, but usually within
> 5 minutes or so), I encounter a segfault when freeing the MMManager object
> specifically. Prior to the segfault, the rest of the iteration happens
> normally and data is still read correctly from the modem. Here is a brief
> backtrace from gdb showing that the segfault happens during g_object_unref():
>
> Thread 6 "XXX" received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 0x73eff410 (LWP 2195)]
> 0x764cb85c in g_type_check_instance_is_fundamentally_a () from
> /lib/libgobject-2.0.so.0
> (gdb) bt
> #0 0x764cb85c in g_type_check_instance_is_fundamentally_a () from
> /lib/libgobject-2.0.so.0
> #1 0x764a43fc in g_object_unref () from /lib/libgobject-2.0.so.0
> #2 0x76b429b4 in ?? () from /lib/libglib-2.0.so.0
>
> And here is the code that I am using to actually do the freeing:
>
> /* Clean up */
> if (modem) {
> g_object_unref (modem);
> }
> if (object) {
> g_object_unref (object);
> }
> if (modems) {
> g_list_free_full (modems, g_object_unref);
> }
> if (manager) {
> g_object_unref (manager);
> }
>
"modems" here is the list returned by the manager with
g_dbus_object_manager_get_objects(). The list contains full
references, so g_list_free_full() is fine.
"object" is an element of that list. Are you really doing a
g_object_ref() of the object when you're iterating? Or are you
unref-ing the "object" without having acquired a full reference?
Remember that you're then doing a g_list_free_full() on the whole
object list. Maybe here's the problem?
"modem" is the Modem interface object which you get with
mm_object_get_modem(), that returns a full reference, so that's fine.
> Where "modem" is an MMModem*, "object" is an MMObject*, "modems" is a GList*,
> and "manager" is an MMManager*. The DBusConnection* exists outside the loop
> and is freed afterwards. The segfault, when it happens, *only* occurs when
> freeing "manager."
>
Could very well be that the "object" keeps an internal reference to
the manager and that's why the crash is happening (double free of
manager).
> In my testing, I tried using g_clear_object(*manager) and encountered the
> same issue. In addition, I compiled my application with it freeing every
> object other than the manager and it ran perfectly fine (other than the
> memory leak) for hours without encountering a segfault.
>
Debugging reference issues is not always trivial, sometimes not even
valgrind can help. In your case, though, the true issue at the end is
a double free and you know which object is the culprit, so it looks
like it may not be difficult to find.
> Do you have any idea what might be causing this? Are there any potential
> workarounds I could use to free that object's memory? I would appreciate any
> advice.
>
If you send the test application source code (doesn't need to be your
full application, just a reproducer) I can give it a look; we should
definitely confirm this is not a problem in libmm-glib itself.
--
Aleksander
https://aleksander.es
#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>
#include <stdint.h>
#include <string.h>
#include <time.h>
#include <glib.h>
#include <libmm-glib.h>
#define POLL_TIME_FIFTEEN_SEC 15
#define ERROR_TIMEOUT_COUNT 20
int main (int argc, char *argv[]) {
int error_count = 0;
GDBusConnection *connection = NULL;
/* Set up DBus connection to use */
connection = g_bus_get_sync (G_BUS_TYPE_SYSTEM, NULL, NULL);
if (connection == NULL) {
printf("%s::%s:%d error: couldn't get bus\n", __FILE__, __func__, __LINE__);
}
for (int i = 0; i < 10; i++) { /* Run a limited number of times so "connection" is eventually freed */
gchar *name_owner = NULL;
GList *modems = NULL;
MMManager *manager = NULL;
MMObject *object = NULL;
MMModem *modem = NULL;
GError *error = NULL;
bool has_error = false;
uint8_t signal_quality = 0;
MMModemState mmm_state = MM_MODEM_STATE_UNKNOWN;
if (error_count > ERROR_TIMEOUT_COUNT) {
printf("%s::%s:%d Errors persisted past timeout period. Exiting...\n", __FILE__, __func__, __LINE__);
break;
}
/* Make sure we still have a DBus connection -- try to re-open it if it is gone or closed */
if (connection == NULL || g_dbus_connection_is_closed(connection)){
printf("%s::%s:%d DBus connection closed or not found\n", __FILE__, __func__, __LINE__);
if (connection != NULL) {
g_object_unref(connection);
}
connection = g_bus_get_sync (G_BUS_TYPE_SYSTEM, NULL, &error);
if (connection == NULL) {
printf("%s::%s:%d error: couldn't get bus: %s\n", __FILE__, __func__, __LINE__, error ? error->message : "unknown error");
has_error = true;
error_count++;
}
}
/* Set up manager */
if (has_error == false) {
manager = mm_manager_new_sync (connection, G_DBUS_OBJECT_MANAGER_CLIENT_FLAGS_DO_NOT_AUTO_START, NULL, &error);
if (manager == NULL) {
printf("%s::%s:%d error: couldn't add new manager to bus: %s\n", __FILE__, __func__, __LINE__, error ? error->message : "unknown error");
has_error = true;
error_count++;
}
}
/* Make sure ModemManager is running */
if (has_error == false) {
name_owner = g_dbus_object_manager_client_get_name_owner (G_DBUS_OBJECT_MANAGER_CLIENT (manager));
if (name_owner == NULL) {
printf("%s::%s:%d error: couldn't find the ModemManager process in the bus\n", __FILE__, __func__, __LINE__);
has_error = true;
error_count++;
} else {
g_free (name_owner);
}
}
/* Get a list of modem objects */
if (has_error == false) {
modems = g_dbus_object_manager_get_objects (G_DBUS_OBJECT_MANAGER (manager));
if (modems == NULL) {
printf("%s::%s:%d error: couldn't find any modem objects in manager\n", __FILE__, __func__, __LINE__);
has_error = true;
error_count++;
}
}
/* There will only be one modem object, if any, so grab the first */
if (has_error == false) {
object = MM_OBJECT (modems->data);
if (object == NULL) {
printf("%s::%s:%d error: object is null\n", __FILE__, __func__, __LINE__);
has_error = true;
error_count++;
}
}
/* Get the modem from the modem object */
if (has_error == false) {
modem = MM_MODEM (mm_object_get_modem (object));
if (modem == NULL) {
printf("%s::%s:%d error: modem is null\n", __FILE__, __func__, __LINE__);
has_error = true;
error_count++;
}
}
/* Get data from modem and do stuff with it */
if (has_error == false) {
/* Check modem state */
mmm_state = mm_modem_get_state(modem);
/* Check signal quality */
signal_quality = mm_modem_get_signal_quality(modem, NULL);
/* Print results */
printf("Signal quality = %d\n", signal_quality);
printf("Modem state = %d\n", mmm_state);
/* If we've gotten this far, reset errors */
error_count = 0;
}
/* Clean up */
printf("Freeing modem\n");
if (modem != NULL) {
g_object_unref (modem);
}
/* Freeing object is unnecessary, as it is a part of the "modems" list */
printf("Freeing modems\n");
if (modems != NULL) {
g_list_free_full (modems, g_object_unref);
}
/* name_owner is already freed above directly after use */
printf("Freeing manager\n");
if (manager != NULL) {
g_object_unref (manager);
}
/* connection is handled separately because it originates outside of the loop */
g_clear_error(&error);
/* printf("Disposing connection\n"); */
/* g_object_run_dispose(G_OBJECT(connection)); */
printf("Done freeing objects\n\n");
/* Zzzzzzzzz... */
sleep(POLL_TIME_FIFTEEN_SEC);
}
if (connection != NULL) {
g_object_unref(connection);
}
}
_______________________________________________
ModemManager-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
