GNOME Bugzilla – Bug 598181
Applet crashes if a killswitch is added after startup
Last modified: 2009-10-13 11:28:48 UTC
If the applet starts with no killswitches installed, the following code in main.c destroys the killswitch object: if (bluetooth_killswitch_has_killswitches (killswitch) == FALSE) { g_object_unref (killswitch); killswitch = NULL; } However, the destructor for the killswitch object (bluetooth_killswitch_finalize) fails to cleanup the io_watch set in its constructor. So when a killswitch is connected later, the event_cb for the killswitch object runs, and attempts to use the now-destructed killswitch object. The result is a crash. The problems here are twofold: * Just because we don't have a killswitch now, doesn't mean we won't get one later (the user may attach a bluetooth interface with one). Thus we shouldn't destroy the killswitch object just because it doesn't have any yet. * The killswitch finalizer fails to fully clean up after the constructor. I've attached a patch against latest git to fix this issue; I'm not too familiar with the code, so please give it a thorough review before applying. Note that this issue is easily reproducible with a USB dongle that includes a killswitch; start the applet with zero bluetooth devices, then plug in the dongle. It's been affecting a number of ubuntu users at https://bugs.edge.launchpad.net/ubuntu/+source/gnome-bluetooth/+bug/445422 , so it's not just theoretical :)
Created attachment 145298 [details] [review] Patch to fix issue (needs review)
I'm pretty sure this isn't enough. We also need to make sure the code in bluetooth-killswitch.c handles killswitches coming and going (I'm pretty sure it doesn't like killswitches going away), and the code handling "killswitch object == NULL) in both the applet and the properties need to be removed/checked.
Unfortunately, I don't have any bluetooth hardware with an actual killswitch (mine apparently claims to have one, but it's permanently in the RF-enable position), so I don't think I'll be able to take this patch any further. :/ In any case, though, the crash needs fixing, so just the chunks that fix the finalizer should be checked in I think (as that is buggy regardless of what other bugs may lurk :)