GNOME Bugzilla – Bug 733957
Shell search provider crashes in gcr_collection_contains()
Last modified: 2014-09-23 08:04:36 UTC
Created attachment 281996 [details] bt Crash, search provider related? If you need more info, or more debug symbols installed, I can probably do it.
Created attachment 282067 [details] [review] search-provider: Comment about hacky stuff going on with scanf
Comment on attachment 282067 [details] [review] search-provider: Comment about hacky stuff going on with scanf I was going to fix this in gcr ... but actually lets just fix it properly in seahorse.
Created attachment 282089 [details] [review] search-provider: Don't assume handles are still valid We were crashing when we got back search provider handles from the shell for objects that no longer existed. We can't just call gcr_collection_contains() as it assumes that it's receiving a valid GObject. So hold a table of all the handles we've returned and update it via weak references.
Giovanni, could you review this patch?
Review of attachment 282089 [details] [review]: At some point I had a patch that used mincore() to check if a pointer was valid or not. I suppose this is a better idea in terms of readability and reliability, at the expense of increased memory usage. Looks good to me anyway.
(In reply to comment #5) > Review of attachment 282089 [details] [review]: > > At some point I had a patch that used mincore() to check if a pointer was valid > or not. Hmmm, there would still be corner cases when that would return invalid results. For example the pointer could be near the end of a page, and the next page might not be mapped in. Or the pointer could be to garbage, and then G_IS_OBJECT() (or similar) would dereferece yet another bad pointer. > I suppose this is a better idea in terms of readability and > reliability, at the expense of increased memory usage. Looks good to me anyway. OK. Thanks for reviewing it. Will merge.
Attachment 282089 [details] pushed as 073a958 - search-provider: Don't assume handles are still valid
Created attachment 284656 [details] backported_fix_crash For now I won't reopen this (maybe I should file another report?). This bugfix was backported to 3.10.x in fedora 20 ( https://admin.fedoraproject.org/updates/seahorse-3.10.2-2.fc20 ), I tested the update and search provider works, it finds my gpg key (only key which is trusted in my keyring), but when I click on it, seahorse segfaults. I'll attach backtrace, but no symbols currently, I can install them if needed.
Symbols would indeed be helpful. Thanks.
Created attachment 286842 [details] bt Sorry for late answer on this issue, I didn't have time to do it, and I upgraded to f21 (where seahorse search provider doesn't work (didn't reported that)). And now I set up a VM with f20, and actually that crash from comment #8 is probably unrelated to changes done about random crashes ... which you fixed. Because I can reproduce it with regular f20 seahorse package without backports. So this is data from f20 seahorse package without fixes mentioned in this bug report (that package got unpushed form updates-testing (I can get same trace from it too (find it in koji ...)), and probably this should go to another bug report. Same situation, search for a gpg key, click on it, and it crashes. (I'm not an expert in getting traces, looks like some symbols are missing, but I installed them with 'debuginfo-install seahorse')
Sorry for this noise, I was looking at the code (3.10 branch) and if I'm not wrong it crashes in this line 'window = GTK_WINDOW (seahorse_widget_get_widget (key_manager, key_manager->name));' and it does that because 'key_manager' is NULL (0x0)? in gdb, "after" the crash, if I say p key_manager it says $1 = (SeahorseWidget *) 0x0 so it actually fails at both code paths to "get" 'key_manager'? (Again sorry for this, I want to learn more, just I'm too limited)
(In reply to comment #11) > Sorry for this noise, I was looking at the code (3.10 branch) and if I'm not > wrong it crashes in this line 'window = GTK_WINDOW (seahorse_widget_get_widget > (key_manager, key_manager->name));' and it does that because 'key_manager' is > NULL (0x0)? > > in gdb, "after" the crash, if I say > p key_manager it says > $1 = (SeahorseWidget *) 0x0 > so it actually fails at both code paths to "get" 'key_manager'? Indeed. Looks like a different crash. Another bug with a backtrace would be most appreciated.
filled bug #737153