After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 598181 - Applet crashes if a killswitch is added after startup
Applet crashes if a killswitch is added after startup
Status: RESOLVED FIXED
Product: gnome-bluetooth
Classification: Core
Component: applet
2.28.x
Other Linux
: Normal normal
: ---
Assigned To: gnome-bluetooth-general-maint@gnome.bugs
gnome-bluetooth-general-maint@gnome.bugs
Depends on:
Blocks:
 
 
Reported: 2009-10-12 18:05 UTC by bdonlan
Modified: 2009-10-13 11:28 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to fix issue (needs review) (3.14 KB, patch)
2009-10-12 18:07 UTC, bdonlan
committed Details | Review

Description bdonlan 2009-10-12 18:05:32 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 :)
Comment 1 bdonlan 2009-10-12 18:07:28 UTC
Created attachment 145298 [details] [review]
Patch to fix issue (needs review)
Comment 2 Bastien Nocera 2009-10-13 01:26:21 UTC
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.
Comment 3 bdonlan 2009-10-13 02:11:04 UTC
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 :)