summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAleksander Morgado <aleksander@aleksander.es>2020-01-10 13:22:33 +0100
committerAleksander Morgado <aleksander@aleksander.es>2020-01-10 14:20:23 +0100
commit3c02f7e9d9f5b87d2aa7ce336e148ad7d9d0d1ab (patch)
tree85c9c78ff5dfe2ddf25bef2196f2ec19fc2d7f1e
parent964167575f6a40ab12a002a24f0d3100025b5287 (diff)
iface-modem-location: fix big memleak when processing location updates
The GVariants that we obtain during the processing of the "previous" dictionary with g_variant_iter_next() are full references, while the GVariants that we obtain processing the input MMLocationXX objects are floating references. The code was working well with the floating references only, as it was assumed that g_variant_builder_add() would take ownership of them as full references; but when the GVariant came from the g_variant_iter_next() processing, g_variant_builder_add() would take a full new extra reference, triggering the memory leak. Fix this, by making sure that we always work with full GVariant references in all cases, converting the floating ones with g_variant_ref_sink() and making sure we explicitly unref them after g_variant_builder_add(). ==3146== 112 (48 direct, 64 indirect) bytes in 1 blocks are definitely lost in loss record 4,532 of 4,887 ==3146== at 0x483877F: malloc (vg_replace_malloc.c:309) ==3146== by 0x50D53DA: g_malloc (gmem.c:99) ==3146== by 0x50F19ED: g_slice_alloc (gslice.c:1024) ==3146== by 0x511D639: g_variant_alloc (gvariant-core.c:486) ==3146== by 0x511D6F9: g_variant_new_from_bytes (gvariant-core.c:529) ==3146== by 0x51146C3: g_variant_new_from_trusted (gvariant.c:326) ==3146== by 0x5115D28: g_variant_new_string (gvariant.c:1264) ==3146== by 0x48EC642: mm_location_3gpp_get_string_variant (mm-location-3gpp.c:294) ==3146== by 0x196350: build_location_dictionary (mm-iface-modem-location.c:162) ==3146== by 0x197E0C: handle_setup_auth_ready (mm-iface-modem-location.c:891) ==3146== by 0x4EC9160: g_task_return_now (gtask.c:1212) ==3146== by 0x4EC92AA: g_task_return (gtask.c:1281) ==3146== 41,658 (35,520 direct, 6,138 indirect) bytes in 740 blocks are definitely lost in loss record 4,887 of 4,887 ==3146== at 0x483877F: malloc (vg_replace_malloc.c:309) ==3146== by 0x50D53DA: g_malloc (gmem.c:99) ==3146== by 0x50F19ED: g_slice_alloc (gslice.c:1024) ==3146== by 0x511E02B: g_variant_get_child_value (gvariant-core.c:1093) ==3146== by 0x5114FA7: g_variant_get_variant (gvariant.c:748) ==3146== by 0x511B196: g_variant_valist_get_nnp (gvariant.c:4934) ==3146== by 0x511B87B: g_variant_valist_get_leaf (gvariant.c:5051) ==3146== by 0x511BFBF: g_variant_valist_get (gvariant.c:5232) ==3146== by 0x511C145: g_variant_valist_get (gvariant.c:5267) ==3146== by 0x511C953: g_variant_iter_next (gvariant.c:5667) ==3146== by 0x1962F5: build_location_dictionary (mm-iface-modem-location.c:128) ==3146== by 0x19659A: notify_gps_location_update (mm-iface-modem-location.c:231) (cherry picked from commit 246fe710b728c7d2637b8a30b7db1901a79fca58)
-rw-r--r--src/mm-iface-modem-location.c21
1 files changed, 18 insertions, 3 deletions
diff --git a/src/mm-iface-modem-location.c b/src/mm-iface-modem-location.c
index 4d4cdb91..2547132a 100644
--- a/src/mm-iface-modem-location.c
+++ b/src/mm-iface-modem-location.c
@@ -161,6 +161,8 @@ build_location_dictionary (GVariant *previous,
if (location_3gpp_value)
g_variant_unref (location_3gpp_value);
location_3gpp_value = mm_location_3gpp_get_string_variant (location_3gpp);
+ if (location_3gpp_value)
+ g_variant_ref_sink (location_3gpp_value);
}
if (location_3gpp_value) {
@@ -168,6 +170,7 @@ build_location_dictionary (GVariant *previous,
"{uv}",
MM_MODEM_LOCATION_SOURCE_3GPP_LAC_CI,
location_3gpp_value);
+ g_variant_unref (location_3gpp_value);
}
/* If a new one given, use it */
@@ -175,39 +178,51 @@ build_location_dictionary (GVariant *previous,
if (location_gps_nmea_value)
g_variant_unref (location_gps_nmea_value);
location_gps_nmea_value = mm_location_gps_nmea_get_string_variant (location_gps_nmea);
+ if (location_gps_nmea_value)
+ g_variant_ref_sink (location_gps_nmea_value);
}
- if (location_gps_nmea_value)
+ if (location_gps_nmea_value) {
g_variant_builder_add (&builder,
"{uv}",
MM_MODEM_LOCATION_SOURCE_GPS_NMEA,
location_gps_nmea_value);
+ g_variant_unref (location_gps_nmea_value);
+ }
/* If a new one given, use it */
if (location_gps_raw) {
if (location_gps_raw_value)
g_variant_unref (location_gps_raw_value);
location_gps_raw_value = mm_location_gps_raw_get_dictionary (location_gps_raw);
+ if (location_gps_raw_value)
+ g_variant_ref_sink (location_gps_raw_value);
}
- if (location_gps_raw_value)
+ if (location_gps_raw_value) {
g_variant_builder_add (&builder,
"{uv}",
MM_MODEM_LOCATION_SOURCE_GPS_RAW,
location_gps_raw_value);
+ g_variant_unref (location_gps_raw_value);
+ }
/* If a new one given, use it */
if (location_cdma_bs) {
if (location_cdma_bs_value)
g_variant_unref (location_cdma_bs_value);
location_cdma_bs_value = mm_location_cdma_bs_get_dictionary (location_cdma_bs);
+ if (location_cdma_bs_value)
+ g_variant_ref_sink (location_cdma_bs_value);
}
- if (location_cdma_bs_value)
+ if (location_cdma_bs_value) {
g_variant_builder_add (&builder,
"{uv}",
MM_MODEM_LOCATION_SOURCE_CDMA_BS,
location_cdma_bs_value);
+ g_variant_unref (location_cdma_bs_value);
+ }
return g_variant_builder_end (&builder);
}