Ticket #58 (closed defect: fixed)

Opened 2 years ago

Last modified 1 year ago

padevchooser: server/sink/source list keeps growing

Reported by: ZlatkO Assigned to: lennart
Priority: low Milestone: 0.9.6
Component: padevchooser Severity: minor
Keywords: padevchooser hash Cc: zlatko@gmx.at

Description

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@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@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.

Attachments

padevchooser-0.9.3-hash-table.diff (433 bytes) - added by ZlatkO on 02/05/07 13:29:34.
Patch to fix the growing server/sink/source list and popups

Change History

02/05/07 13:29:34 changed by ZlatkO

  • attachment padevchooser-0.9.3-hash-table.diff added.

Patch to fix the growing server/sink/source list and popups

05/23/07 17:51:17 changed by lennart

  • component changed from clients to padevchooser.

05/23/07 17:51:23 changed by lennart

  • status changed from new to assigned.

05/27/07 01:34:34 changed by lennart

  • status changed from assigned to closed.
  • resolution set to fixed.
  • milestone set to 0.9.6.

The problem is that the PA browsing API has been designed before I worked on Avahi. The old API doesn't know the difference between IPv4 and IPv6. The safest fix for now is probably to disable Ipv6 for this API. And that is exactly what r1453 does.

Eventually the browsing API should go away entirely. People can use the Avahi API directly. There is no need to abstract this away.