Commit 60c61d68 authored by Jim Nelson's avatar Jim Nelson

#3152: Fixes crasher where two different Hirigana tag names collated as equal...

#3152: Fixes crasher where two different Hirigana tag names collated as equal when they plainly were not.  Also fixes a bug I discovered while investigating that a soft assertion was triggered when pages were destroyed.
parent ae16f579
......@@ -1215,7 +1215,7 @@ public class LibraryWindow : AppWindow {
assert(page_stub != null);
sidebar.rename(page_stub.get_marker(), tag.get_name());
sidebar.sort_branch(tags_marker, tag_page_comparator);
sidebar.sort_branch(tags_marker, TagPage.Stub.comparator);
}
sidebar.cursor_changed.connect(on_sidebar_cursor_changed);
......@@ -1288,13 +1288,6 @@ public class LibraryWindow : AppWindow {
return new_dir;
}
private int64 tag_page_comparator(void *a, void *b) {
Tag atag = ((TagPage.Stub *) a)->tag;
Tag btag = ((TagPage.Stub *) b)->tag;
return strcmp(atag.get_name_collate_key(), btag.get_name_collate_key());
}
private void add_tag_page(Tag tag) {
if (tags_marker == null) {
tags_marker = sidebar.add_toplevel_grouping(_("Tags"), new GLib.ThemedIcon(Resources.ICON_TAGS),
......@@ -1302,7 +1295,7 @@ public class LibraryWindow : AppWindow {
}
TagPage.Stub stub = TagPage.create_stub(tag);
sidebar.insert_child_sorted(tags_marker, stub, tag_page_comparator);
sidebar.insert_child_sorted(tags_marker, stub, TagPage.Stub.comparator);
tag_map.set(tag, stub);
}
......
......@@ -219,7 +219,11 @@ public abstract class Page : Gtk.ScrolledWindow, SidebarPage {
// toolbars (as of yet) are not created by the UI Manager and need to be destroyed
// explicitly
toolbar.destroy();
// halt any pending callbacks
if (update_actions_scheduler != null)
update_actions_scheduler.cancel();
is_destroyed = true;
base.destroy();
......
......@@ -5,7 +5,8 @@
*/
public class TagSourceCollection : ContainerSourceCollection {
private Gee.HashMap<string, Tag> name_map = new Gee.HashMap<string, Tag>();
private Gee.HashMap<string, Tag> name_map = new Gee.HashMap<string, Tag>(Tag.hash_name_string,
Tag.equal_name_strings);
private Gee.HashMap<MediaSource, Gee.List<Tag>> source_map =
new Gee.HashMap<MediaSource, Gee.List<Tag>>();
private Gee.HashMap<MediaSource, Gee.SortedSet<Tag>> sorted_source_map =
......@@ -79,7 +80,7 @@ public class TagSourceCollection : ContainerSourceCollection {
if (tags == null)
return null;
Gee.SortedSet<Tag> copy = new Gee.TreeSet<Tag>(compare_tag_name);
Gee.SortedSet<Tag> copy = new Gee.TreeSet<Tag>(Tag.compare_names);
copy.add_all(tags);
return copy;
......@@ -159,10 +160,6 @@ public class TagSourceCollection : ContainerSourceCollection {
base.notify_items_altered(map);
}
private static int compare_tag_name(void *a, void *b) {
return strcmp(((Tag *) a)->get_name_collate_key(), ((Tag *) b)->get_name_collate_key());
}
protected override void notify_container_contents_added(ContainerSource container,
Gee.Collection<DataSource> added, bool relinking) {
Tag tag = (Tag) container;
......@@ -180,7 +177,7 @@ public class TagSourceCollection : ContainerSourceCollection {
Gee.SortedSet<Tag>? sorted_tags = sorted_source_map.get(source);
if (sorted_tags == null) {
sorted_tags = new Gee.TreeSet<Tag>(compare_tag_name);
sorted_tags = new Gee.TreeSet<Tag>(Tag.compare_names);
sorted_source_map.set(source, sorted_tags);
}
......@@ -313,7 +310,7 @@ public class Tag : DataSource, ContainerSource, Proxyable, Indexable {
private TagRow row;
private ViewCollection media_views;
private string? name_collate_key = null;
private string? name_collation_key = null;
private bool unlinking = false;
private bool relinking = false;
private string? indexable_keywords = null;
......@@ -435,6 +432,22 @@ public class Tag : DataSource, ContainerSource, Proxyable, Indexable {
public static void terminate() {
}
public static int compare_names(void *a, void *b) {
Tag *atag = (Tag *) a;
Tag *btag = (Tag *) b;
return String.precollated_compare(atag->get_name(), atag->get_name_collation_key(),
btag->get_name(), btag->get_name_collation_key());
}
public static uint hash_name_string(void *a) {
return String.collated_hash(a);
}
public static bool equal_name_strings(void *a, void *b) {
return String.collated_equals(a, b);
}
// Returns a Tag for the name, creating a new empty one if it does not already exist.
// name should have already been prepared by prep_tag_name.
public static Tag for_name(string name) {
......@@ -505,11 +518,11 @@ public class Tag : DataSource, ContainerSource, Proxyable, Indexable {
return row.name;
}
public string get_name_collate_key() {
if (name_collate_key == null)
name_collate_key = row.name.collate_key();
public string get_name_collation_key() {
if (name_collation_key == null)
name_collation_key = row.name.collate_key();
return name_collate_key;
return name_collation_key;
}
public override string to_string() {
......@@ -665,7 +678,7 @@ public class Tag : DataSource, ContainerSource, Proxyable, Indexable {
}
row.name = new_name;
name_collate_key = null;
name_collation_key = null;
update_indexable_keywords();
......
......@@ -31,6 +31,10 @@ public class TagPage : CollectionPage {
protected override Page construct_page() {
return new TagPage(tag);
}
public static int64 comparator(void *a, void *b) {
return Tag.compare_names(((Stub *) a)->tag, ((Stub *) b)->tag);
}
}
private Tag tag;
......
......@@ -256,9 +256,7 @@ public delegate void OneShotCallback();
public class OneShotScheduler {
private string name;
private OneShotCallback callback;
private bool scheduled = false;
private bool reschedule = false;
private bool cancelled = false;
private uint scheduled = 0;
public OneShotScheduler(string name, OneShotCallback callback) {
this.name = name;
......@@ -270,78 +268,46 @@ public class OneShotScheduler {
debug("DTOR: OneShotScheduler for %s", name);
#endif
cancelled = true;
cancel();
}
public bool is_scheduled() {
return scheduled;
return scheduled != 0;
}
public void at_idle() {
if (scheduled)
return;
scheduled = true;
cancelled = false;
Idle.add(callback_wrapper);
at_priority_idle(Priority.DEFAULT_IDLE);
}
public void at_priority_idle(int priority) {
if (scheduled)
return;
scheduled = true;
cancelled = false;
Idle.add_full(priority, callback_wrapper);
if (scheduled == 0)
scheduled = Idle.add_full(priority, callback_wrapper);
}
public void after_timeout(uint msec, bool reschedule) {
if (scheduled) {
if (reschedule)
this.reschedule = true;
return;
}
scheduled = true;
cancelled = false;
Timeout.add(msec, callback_wrapper);
priority_after_timeout(Priority.DEFAULT, msec, reschedule);
}
public void priority_after_timeout(int priority, uint msec, bool reschedule) {
if (scheduled) {
if (reschedule)
this.reschedule = true;
if (scheduled != 0 && !reschedule)
return;
}
scheduled = true;
cancelled = false;
Timeout.add_full(priority, msec, callback_wrapper);
if (scheduled != 0)
Source.remove(scheduled);
scheduled = Timeout.add_full(priority, msec, callback_wrapper);
}
public void cancel() {
cancelled = true;
reschedule = false;
scheduled = false;
if (scheduled == 0)
return;
Source.remove(scheduled);
scheduled = 0;
}
private bool callback_wrapper() {
if (cancelled) {
cancelled = false;
scheduled = false;
return false;
}
if (reschedule) {
reschedule = false;
return true;
}
scheduled = false;
scheduled = 0;
callback();
return false;
......
......@@ -131,5 +131,68 @@ public string strip_leading_zeroes(string str) {
return stripped.str;
}
public string to_hex_string(string str) {
StringBuilder builder = new StringBuilder();
uint8 *data = (uint8 *) str;
while (*data != 0)
builder.append_printf("%02Xh%s", *data++, (*data != 0) ? " " : "");
return builder.str;
}
// A note on the collated_* and precollated_* methods:
//
// A bug report (http://trac.yorba.org/ticket/3152) indicated that two different Hirigana characters
// as Tag names would trigger an assertion. Investigation showed that the characters' collation
// keys computed as equal when the locale was set to anything but the default locale (C) or
// Japanese. A related bug was that another hash table was using str_equal, which does not use
// collation, meaning that in one table the strings were seen as the same and in another as
// different.
//
// The solution we arrived at is to use collation whenever possible, but if two strings have the
// same collation, then fall back on strcmp(), which looks for byte-for-byte comparisons. Note
// that this technique requires that both strings have been properly composed (use
// prepare_input_text() for that task) so that equal UTF-8 strings are byte-for-byte equal as
// well.
// See note above.
public uint collated_hash(void *ptr) {
string str = (string) ptr;
return str_hash(str.collate_key());
}
// See note above.
public uint precollated_hash(void *ptr) {
return str_hash(ptr);
}
// See note above.
public int collated_compare(void *a, void *b) {
string astr = (string) a;
string bstr = (string) b;
int result = astr.collate(bstr);
return (result != 0) ? result : strcmp(astr, bstr);
}
// See note above.
public int precollated_compare(string astr, string akey, string bstr, string bkey) {
int result = strcmp(akey, bkey);
return (result != 0) ? result : strcmp(astr, bstr);
}
// See note above.
public bool collated_equals(void *a, void *b) {
return collated_compare(a, b) == 0;
}
// See note above.
public bool precollated_equals(string astr, string akey, string bstr, string bkey) {
return precollated_compare(astr, akey, bstr, bkey) == 0;
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment