The Sound Menu

implicitly-generated variants are leaked

Bug #1244468 reported by Charles Kerr on 2013-10-24

This bug affects 1 person

	Status	Importance	Assigned to
Bluetooth Menu	Fix Released	Medium	Charles Kerr
The Sound Menu	Fix Released	Medium	Charles Kerr
Vala	Fix Released	Medium	gnome-bugs #710882
indicator-bluetooth (Ubuntu)	Fix Released	Undecided	Unassigned
indicator-sound (Ubuntu)	Fix Released	Undecided	Unassigned

Bug Description

The implicitly-created GVariant is leaked in Service's volume_changed() function. We can fix it by making the variant explicit. But, should this be reported upstream as a Vala bug?

One line diff:
- volume_action.set_state (volume);
+ volume_action.set_state (new Variant.double (volume));

C code generated from "volume_action.set_state (volume)":

  static GVariant* _variant_new4 (gdouble value) {
    return g_variant_ref_sink (g_variant_new_double (value));
  }
  static void indicator_sound_service_volume_changed (IndicatorSoundService* self, gdouble volume) {
    ...
    _tmp3_ = volume;
   _tmp4_ = _variant_new4 (_tmp3_);
    g_simple_action_set_state (volume_action, _tmp4_);
    ...
  }

C code generated from "volume_action.set_state (new Variant.double (volume))":

  static void indicator_sound_service_volume_changed (IndicatorSoundService* self, gdouble volume) {
    ...
    _tmp3_ = volume;
    _tmp4_ = g_variant_new_double (_tmp3_);
    g_variant_ref_sink (_tmp4_);
    _tmp5_ = _tmp4_;
    g_simple_action_set_state (volume_action, _tmp5_);
    _g_variant_unref0 (_tmp5_);
    ...
  }

See original description

Related branches

lp:~charlesk/indicator-sound/lp-1244468

Merged into lp:indicator-sound/14.04 at revision 395

Lars Karlitski (community): Approve on 2013-10-25

PS Jenkins bot (community): Approve (continuous-integration) on 2013-10-25

lp:~charlesk/indicator-bluetooth/lp-1244468

Merged into lp:indicator-bluetooth/14.04 at revision 74

Lars Karlitski (community): Approve on 2013-10-25

PS Jenkins bot (community): Approve (continuous-integration) on 2013-10-25

lp:ubuntu/trusty-proposed/indicator-bluetooth

lp:ubuntu/trusty-proposed/indicator-sound

Charles Kerr (charlesk) on 2013-10-24

Changed in indicator-sound:
importance:	Undecided → Medium

Revision history for this message

Charles Kerr (charlesk) wrote on 2013-10-24:

Valgrind log of described leak Edit (3.4 KiB, text/plain)

description:

updated

Charles Kerr (charlesk) on 2013-10-25

summary:

- GVariants leaked in service.vala's volume_changed()
+ implicitly-generated variants are leaked

Charles Kerr (charlesk) on 2013-10-25

Changed in indicator-bluetooth:
importance:	Undecided → Medium
Changed in indicator-sound:
status:	New → In Progress
Changed in indicator-bluetooth:
status:	New → In Progress
assignee:	nobody → Charles Kerr (charlesk)
Changed in indicator-sound:
assignee:	nobody → Charles Kerr (charlesk)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-10-25:

Fix committed into lp:indicator-bluetooth at revision 74, scheduled for release in indicator-bluetooth, milestone Unknown

Changed in indicator-bluetooth:
status:	In Progress → Fix Committed

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-10-25:

Fix committed into lp:indicator-sound at revision 395, scheduled for release in indicator-sound, milestone Unknown

Changed in indicator-sound:
status:	In Progress → Fix Committed

Bug Watch Updater (bug-watch-updater) on 2013-10-25

Changed in vala:
importance:	Unknown → Medium
status:	Unknown → New

Bug Watch Updater (bug-watch-updater) on 2013-10-26

Changed in vala:
status:	New → Fix Released

Revision history for this message

Launchpad Janitor (janitor) wrote on 2013-10-29:

This bug was fixed in the package indicator-bluetooth - 0.0.6+14.04.20131029.1-0ubuntu1

---------------
indicator-bluetooth (0.0.6+14.04.20131029.1-0ubuntu1) trusty; urgency=low

  [ Dmitrijs Ledkovs ]
  * Define "ubiquity" indicator profile, reusing the greeter object. (LP:
    #1241539)

  [ Charles Kerr ]
  * explicitly instantiate GVariants to avoid leaking them. See the
    description in bug #1244468 for before & after C code and more
    information. (LP: #1244468)

[ Ted Gould ]
* Adding a phone_greeter profile.

  [ Lars Uebernickel ]
  * Desktop menu: mark "Visible" menu item label as translatable. (LP:
    #1242038)

[ Ubuntu daily release ]
* Automatic snapshot from revision 75
-- Ubuntu daily release <email address hidden> Tue, 29 Oct 2013 10:31:12 +0000

Changed in indicator-bluetooth (Ubuntu):
status:	New → Fix Released

Revision history for this message

Launchpad Janitor (janitor) wrote on 2013-10-29:

This bug was fixed in the package indicator-sound - 12.10.2+14.04.20131029.1-0ubuntu1

---------------
indicator-sound (12.10.2+14.04.20131029.1-0ubuntu1) trusty; urgency=low

  [ Dmitrijs Ledkovs ]
  * Define "ubiquity" indicator profile, reusing the greeter object. (LP:
    #1241539)

  [ Charles Kerr ]
  * When we can't connect to pulse, wait a moment before retrying. (LP:
    #1244010)
  * In Service's volume_changed() function, explicitly instantiate the
    GVariant to avoid leaking it. See the description in bug #1244468
    for before & after C code and more information. (LP: #1244468)
  * Test that MediaPlayer's icon property isn't NULL before we serialize
    it. (LP: #1229612)

  [ Sebastien Bacher ]
  * Don't special case Unity sessions, that's not needed since gnome-
    control-center is doing the right thing, the session name used was
    also wrong and couldn't work (lp: #1239545). (LP: #1239545)

  [ Lars Uebernickel ]
  * Only display running players in the phone's sound menu The design
    says to only display the _one_ running player. This will have that
    effect, as there's only one player allowed to run at a time on the
    phone.
  * VolumeControl: don't pass vala closures into libpulse Libpulse's
    vala bindings don't specify that callbacks passed into some of its
    functions (e.g., get_server_info) can be called after the function
    returns. Vala thus frees closure data after these functions return.
    This can't easily be fixed in the bindings, because libpulse doesn't
    provide variants of these functions with destroy_notifies. This
    patch works around this problem by only passing non-closure
    functions into libpulse. (LP: #1244386)
  * Depend on vala 0.20 and use its GIcon.serialize().

[ Ted Gould ]
* Adding a phone_greeter profile.

[ Ubuntu daily release ]
* Automatic snapshot from revision 399
-- Ubuntu daily release <email address hidden> Tue, 29 Oct 2013 10:32:48 +0000