Comment 4 for bug 414407

We got several duplicate reports about dk-disks crashing in device_remove(), apparently when trying to remove a NULL key from the hash tables:

#0 IA__g_str_hash (v=0x0) at /build/buildd/glib2.0-2.21.4/glib/gstring.c:99
 p = (const signed char *) 0x0
 h = <value optimized out>
#1 0x00926d97 in g_hash_table_remove_internal (hash_table=0x8227e90,
    key=0x0, notify=1) at /build/buildd/glib2.0-2.21.4/glib/ghash.c:195
 node = <value optimized out>
 node_index = <value optimized out>
 __PRETTY_FUNCTION__ = "g_hash_table_remove_internal"
#2 0x0804dc53 in device_remove (daemon=0x8231c08, d=<value optimized out>)
    at devkit-disks-daemon.c:748
 device = <value optimized out>
 native_path = <value optimized out>
 __PRETTY_FUNCTION__ = "device_remove"
[...]
(See http://launchpadlibrarian.net/30388984/ThreadStacktrace.txt for the full trace).

So it seems that one of device->priv->{native_path,device_file,object_path,dev} is NULL. Unfortunately the stack trace isn't precise enough to tell which one in particular. Most duplicates reported that this happens on trying to mount an encrypted drive.

One could just paper over this by checking for NULL keys before trying to remove from the hash table. It's probably a good idea for robustness anyway, but it would paper over the fact that something in the internal state keeping is wrong. These properties aren't queried on device removal, but on addition, so it doesn't sound like a race condition on already lost data on removal.

native_path is already set in devkit_disks_device_new(), so it's unlikely to be NULL. However, object_path is set in register_disks_device(), and device_file and dev in update_info(). So there is one possible path which could lead to this:

        devkit_disks_device_set_device_file (device, g_udev_device_get_device_file (device->priv->d));
        if (device->priv->device_file == NULL) {
                g_warning ("No device file for %s", device->priv->native_path);
                goto out;
        }

So perhaps device_add() shouldn't add such invalid ones to the hash tables, or devices which don't have all four data fields should be discarded at all?