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 735746 - Guard against failure to resolve IDs
Guard against failure to resolve IDs
Status: RESOLVED FIXED
Product: gnome-photos
Classification: Applications
Component: general
3.13.x
Other Linux
: Normal normal
: ---
Assigned To: GNOME photos maintainer(s)
GNOME photos maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2014-08-31 13:49 UTC by Ankur Sinha (FranciscoD)
Modified: 2014-09-17 18:58 UTC
See Also:
GNOME target: 3.14
GNOME version: ---


Attachments
tracker-change-monitor: Plug a memory leak (3.86 KB, patch)
2014-09-17 16:06 UTC, Debarshi Ray
committed Details | Review
tracker-change-monitor: Guard against failure to resolve IDs (1.33 KB, patch)
2014-09-17 16:06 UTC, Debarshi Ray
none Details | Review

Description Ankur Sinha (FranciscoD) 2014-08-31 13:49:49 UTC
From downstream bug report: https://bugzilla.redhat.com/show_bug.cgi?id=1128441

Truncated backtrace:
Thread no. 1 (10 frames)
 #0 g_str_hash at ghash.c:1802
 #1 g_hash_table_lookup_node at ghash.c:368
 #2 g_hash_table_lookup at ghash.c:1092
 #3 photos_tracker_change_monitor_add_event at photos-tracker-change-monitor.c:129
 #4 photos_tracker_change_monitor_send_events at photos-tracker-change-monitor.c:162
 #5 photos_tracker_change_monitor_cursor_next at photos-tracker-change-monitor.c:203
 #6 g_simple_async_result_complete at gsimpleasyncresult.c:763
 #7 complete_in_idle_cb_for_thread at gsimpleasyncresult.c:832
 #11 g_main_context_iteration at gmain.c:3795
 #12 g_application_run at gapplication.c:2219

gnome-photos-3.13.4-2.fc21.x86_64

More information is available in the downstream report.
Comment 1 Matthias Clasen 2014-08-31 15:24:04 UTC
Putting crashes on the blocker list for now, to raise some attention.
Comment 2 Debarshi Ray 2014-09-17 16:06:27 UTC
Created attachment 286400 [details] [review]
tracker-change-monitor: Plug a memory leak
Comment 3 Debarshi Ray 2014-09-17 16:06:59 UTC
Created attachment 286401 [details] [review]
tracker-change-monitor: Guard against failure to resolve IDs
Comment 4 Debarshi Ray 2014-09-17 16:37:10 UTC
A braindump:

The crash is in the code that listens to the graph-updated signal from Tracker. This signal is emitted when there is some change in Tracker's database. For example, this can happen when a new file was created or an old one deleted, or when some new metadata like nie:title was added.

Each event is represented in graph-updated as (graph-id, subject-id, predicate-id, object-id). These IDs are numbers that can be resolved to their more common string representations by a query like:
  "SELECT tracker:uri(id1) tracker:uri(id2) ... tracker:uri(idN) {}"

We resolve only the subject and predicate IDs and have a hash table mapping them to their string values. For some reason one of the subject IDs did not resolve leading to 'urn' being NULL in photos_tracker_change_monitor_add_event. (NB: I could not trigger this by cp-ing and rm-ing a few images.)

Question is whether it is expected behaviour from Tracker or not.

I have added a guard against such scenerios in the above patch, and would like to run this past the Tracker folks before committing it.
Comment 5 Debarshi Ray 2014-09-17 18:35:09 UTC
In #tracker on GIMPNet:

18:33 <rishi> pvanhoof: I guess I will go ahead with a "if (G_UNLIKELY (urn ==  
      NULL))" to ignore events with unresolvable IDs.
18:33 <pvanhoof> yah
18:33 <rishi> Sounds like a weird / edge case to me.
18:33 <pvanhoof> sounds like correct
...
18:34 <rishi> pvanhoof: Ok. Is that supposed to mean that "IDs should always    
      resolve"?
18:34 <pvanhoof> yes IDs that you get from us should always resolve
18:34 <rishi> Or does that mean "IDs are never recycled"?
18:35 <pvanhoof> both
18:35 <pvanhoof> But you need to get them from us
18:35 <pvanhoof> random() is not ok :)