diff options
author | Aleksander Morgado <aleksander@aleksander.es> | 2020-01-10 13:22:33 +0100 |
---|---|---|
committer | Aleksander Morgado <aleksander@aleksander.es> | 2020-01-10 14:20:23 +0100 |
commit | 3c02f7e9d9f5b87d2aa7ce336e148ad7d9d0d1ab (patch) | |
tree | 85c9c78ff5dfe2ddf25bef2196f2ec19fc2d7f1e | |
parent | 964167575f6a40ab12a002a24f0d3100025b5287 (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.c | 21 |
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); } |