[pulseaudio-tickets] [PulseAudio] #58: padevchooser: server/sink/source list keeps growing

[PulseAudio]

#58: padevchooser: server/sink/source list keeps growing
-------------------------------+--------------------------------------------
 Reporter:  ZlatkO             |       Owner:  lennart
     Type:  defect             |      Status:  new    
 Priority:  low                |   Milestone:         
Component:  clients            |    Severity:  minor  
 Keywords:  padevchooser hash  |  
-------------------------------+--------------------------------------------
 ''Note: I didn't find a hint on where to report padevchooser bugs, so I
 decided to do it here.''

 Version info: padevchooser-0.9.3, pulseaudio-0.9.5, glib-2.12.9.

 Entries for the server, sink and source hash tables do not get removed
 properly when the corresponding service disappears, so the lists (and the
 related server, sink and source popups on right-click) keep growing over
 time.

 This happens because module-zeroconf-publish publishes both IPv4 and IPv6
 services, which both have the same name, and thus the same key when
 inserted into the hash table:

 {{{
 [zlatko at disclosure]:~$ avahi-browse -a -t | grep -i pulse | sort
 + eth0 IPv4 disclosure                                    PulseAudio Sound
 Server local
 + eth0 IPv4 input_alsa_headset on disclosure              PulseAudio Sound
 Source local
 + eth0 IPv4 input_alsa_intel on disclosure                PulseAudio Sound
 Source local
 + eth0 IPv4 input_oss_headset on disclosure               PulseAudio Sound
 Source local
 + eth0 IPv4 input_oss_intel on disclosure                 PulseAudio Sound
 Source local
 + eth0 IPv4 output_alsa_headset on disclosure             PulseAudio Sound
 Sink local
 + eth0 IPv4 output_alsa_intel on disclosure               PulseAudio Sound
 Sink local
 + eth0 IPv4 output_oss_headset on disclosure              PulseAudio Sound
 Sink local
 + eth0 IPv4 output_oss_intel on disclosure                PulseAudio Sound
 Sink local
 + eth0 IPv6 disclosure                                    PulseAudio Sound
 Server local
 + eth0 IPv6 input_alsa_headset on disclosure              PulseAudio Sound
 Source local
 + eth0 IPv6 input_alsa_intel on disclosure                PulseAudio Sound
 Source local
 + eth0 IPv6 input_oss_headset on disclosure               PulseAudio Sound
 Source local
 + eth0 IPv6 input_oss_intel on disclosure                 PulseAudio Sound
 Source local
 + eth0 IPv6 output_alsa_headset on disclosure             PulseAudio Sound
 Sink local
 + eth0 IPv6 output_alsa_intel on disclosure               PulseAudio Sound
 Sink local
 + eth0 IPv6 output_oss_headset on disclosure              PulseAudio Sound
 Sink local
 + eth0 IPv6 output_oss_intel on disclosure                PulseAudio Sound
 Sink local
 [zlatko at disclosure]:~$
 }}}

 So, for example, browse_cb() on startup finds the PA_BROWSE_NEW_SINK
 opcode for the IPv4 version of "output_alsa_intel on disclosure", and adds
 the record to the hash table using g_hash_table_insert(), with
 "output_alsa_intel on disclosure" as key. At this point,
 g_hash_table_lookup() would return a pointer to that entry, and all would
 be well.

 Shortly after that, the same happens with the IPv6 version. It also gets
 added to the hash table using g_hash_table_insert() with the same key,
 "output_alsa_intel on disclosure". From this point on, all subsequent
 calls to g_hash_table_lookup() for that key return NULL. I don't know
 whether this is due to a bug in glib, or due to the fact that the hash
 tables are generated with g_hash_table_new_full() and
 menu_item_info_free() specified as the value_destroy_func.

 Meanwhile, sinks and sources might disappear again due to the automatic
 unloading of idle modules. They don't get removed from padevchooser's
 list, though, because remove_menu_item_info() simply returns without doing
 anything if g_hash_table_lookup() fails (which it always does at this
 point, see above). As soon as the modules are needed again and get
 (auto-)reloaded, the associated sinks also reappear, and the whole cycle
 starts all over. And padevchooser's list (and popups) grows and grows and
 grows ...

 The enclosed trivial patch fixes the problem. An implicit assumption of
 this patch is that if either one of the IPv4 or IPv6 servers/sinks/sources
 disappear, then the other one disappears as well. This seems to be a valid
 assumption within padevchooser's scope, as there is currently no
 distinction between IPv4 and IPv6 services anyway - otherwise this bug
 would not even exist. :-)

 Possible alternative solutions would be to also implement a
 key_destroy_func that takes care of properly updating the hash table, or
 to include this functionality in remove_menu_item_info(), or to use
 g_hash_table_lookup_extended() to be be able to distinguish between non-
 existing keys and existing keys with a NULL value. I don't know if it's
 worth it, though, it seems a bit like overkill to me.

-- 
Ticket URL: <http://pulseaudio.org/ticket/58>
PulseAudio <http://pulseaudio.org/>
The PulseAudio Sound Server