GNOME Bugzilla – Bug 724927
Crash when doing any search with Maps 3.11.90 on Fedora Rawhide
Last modified: 2014-02-28 12:59:44 UTC
Well, just as in description. I can reliably crash gnome-maps-3.11.90-1.fc21 , on two machines, just by trying to do a search. Doesn't matter what I search for (or at least 'vancouver' and 'monkeys' both crash it). The spinner rotates briefly, then it stops, then a few seconds later the app crashes.
https://retrace.fedoraproject.org/faf/reports/366248/ is the FAF report. I tried to file it via ABRT, but it decides the backtrace is unusable for some reason.
Hi, thanks for the report! The trace points to a function in geocode-glib. Could you try running 'make check', in geocode-glib to make sure search works there?
I'm running Fedora Rawhide packages, not jhbuild. I'd have to hack up the Fedora package build. I can do that, but I'll have to get around to it. Might be next week.
(In reply to comment #2) > Hi, thanks for the report! > > The trace points to a function in geocode-glib. Could you try running 'make > check', in geocode-glib to make sure search works there? Make check is broken in geocode-glib afaict. From trace its pretty obvious that crash is in geocode-glib.
BTW, I can reproduce the issue here in my jhbuild env.
Ok, thats "good", ill look into it tomorrow.
(In reply to comment #6) > Ok, thats "good", ill look into it tomorrow. I did a bit of investigation. I can reproduce it even with 3.10.x of geocode-glib and Maps so its not a regression in these at least. I failed to reproduce it against the same version of system-installed (Fedora 20) Maps and geocode-glib outside jhbuild. The issue is return value of json_reader_list_members being NULL. So its most likely either json-glib or further down the stack. We'll need a testcase though.
There were changes in json-glib. You might have been relying on the old buggy behaviour. See: https://bugzilla.gnome.org/show_bug.cgi?id=723428
(This might be a case for installed tests, so that they're run daily as part of gnome-continuous).
Created attachment 270078 [details] [review] reverse: fix json-glib usage This patch fixes two issues with the json-glib usage. * Do not atempt to read a string value when the node does not hold a string. * Make sure to call _end_member() when _read_member() fails. We were relying on the old buggy behavour of json-glib, now when that has been fixed up our code blows up.
Review of attachment 270078 [details] [review]: I would put these two separate changes in separate patches but they are small so no strong feelings. ::: geocode-glib/geocode-reverse.c @@ +136,3 @@ + JsonNode *node = json_reader_get_value (reader); + if (json_node_get_value_type (node) == G_TYPE_STRING) { + value = json_reader_get_string_value (reader); once we have the node, we can just get the value directly from node itself using json_node_get_string(). Haven't checked but it might be more efficient even? @@ +137,3 @@ + if (json_node_get_value_type (node) == G_TYPE_STRING) { + value = json_reader_get_string_value (reader); + if (value && *value == '\0') Can it still be NULL if type is reported to be G_TYPE_STRING.
Review of attachment 270078 [details] [review]: ::: geocode-glib/geocode-reverse.c @@ +136,3 @@ + JsonNode *node = json_reader_get_value (reader); + if (json_node_get_value_type (node) == G_TYPE_STRING) { + value = json_reader_get_string_value (reader); Yeah, agreed. @@ +137,3 @@ + if (json_node_get_value_type (node) == G_TYPE_STRING) { + value = json_reader_get_string_value (reader); + if (value && *value == '\0') Yeah, it returns the type that the node has been initialized with. json_node_init_string has allow-none on its string value argument.
(In reply to comment #11) > Review of attachment 270078 [details] [review]: > > I would put these two separate changes in separate patches but they are small > so no strong feelings. > I think I want it in one, since both are needed to avoid crashing and burning. Improves bisectability somewhat.
Created attachment 270096 [details] [review] reverse: fix json-glib usage This patch fixes two issues with the json-glib usage. * Do not atempt to read a string value when the node does not hold a string. * Make sure to call _end_member() when _read_member() fails. We were relying on the old buggy behavour of json-glib, now when that has been fixed up our code blows up.
You'll need to bump the required version in configure.ac as well.
Created attachment 270112 [details] [review] reverse: fix json-glib usage and bump req. version This patch fixes two issues with our json-glib usage. * Do not atempt to read a string value when the node does not hold a string. * Make sure to call _end_member() when _read_member() fails. Without these fixes our code will result in segmentation fault. We were relying on the old buggy behaviour of json-glib that has been fixed up in: 501c9fb reader: When a read() fails, don't track back on end() This patch will also bump the required version of json-glib to make sure the commit referenced above is included.
(In reply to comment #13) > (In reply to comment #11) > > Review of attachment 270078 [details] [review] [details]: > > > > I would put these two separate changes in separate patches but they are small > > so no strong feelings. > > > > I think I want it in one, since both are needed to avoid crashing and burning. > Improves bisectability somewhat. Sure, as said no strong feelings as we desperately need this ATM but for future reference, changes are ideally divided into patches based on what is a logical change rather than the criterias you mentioned.
Review of attachment 270112 [details] [review]: Now there is 3 logical changes in one patch but otherwise all good. The shortlog says 'bump req version' but not mentioning what. I'd at least put the version bump in a seperate patch. Up to you.
Created attachment 270132 [details] [review] reader: fix json-glib usage This patch fixes two issues with our json-glib usage. * Do not atempt to read a string value when the node does not hold a string. * Make sure to call _end_member() when _read_member() fails. Without these fixes our code will result in segmentation fault. We were relying on the old buggy behaviour of json-glib that has been fixed up in: 501c9fb reader: When a read() fails, don't track back on end()
Created attachment 270133 [details] [review] Bump the required version of json-glib to 0.99.2 The behaviour of error handling in _end_element() and _end_member() has changed in json-glib. We need the fixed up version.
Review of attachment 270132 [details] [review]: ack
Review of attachment 270133 [details] [review]: ack
Attachment 270132 [details] pushed as 67231f8 - reader: fix json-glib usage Attachment 270133 [details] pushed as 5da9a2c - Bump the required version of json-glib to 0.99.2
*** Bug 725394 has been marked as a duplicate of this bug. ***