diff --git a/linker/linked_list.h b/linker/linked_list.h index e51eb9cb0..14fe1e58d 100644 --- a/linker/linked_list.h +++ b/linker/linked_list.h @@ -88,24 +88,50 @@ class LinkedList { template void for_each(F&& action) { for (LinkedListEntry* e = head_; e != nullptr; e = e->next) { - if (e->element != nullptr) { - action(e->element); - } + action(e->element); } } template - void remove_if(F&& predicate) { - for (LinkedListEntry* e = head_; e != nullptr; e = e->next) { - if (e->element != nullptr && predicate(e->element)) { - e->element = nullptr; + void remove_if(F predicate) { + for (LinkedListEntry* e = head_, *p = nullptr; e != nullptr;) { + if (predicate(e->element)) { + LinkedListEntry* next = e->next; + if (p == nullptr) { + head_ = next; + } else { + p->next = next; + } + Allocator::free(e); + e = next; + } else { + p = e; + e = e->next; } } } - bool contains(const T* el) { + size_t size() const { + size_t sz = 0; for (LinkedListEntry* e = head_; e != nullptr; e = e->next) { - if (e->element != nullptr && e->element == el) { + ++sz; + } + + return sz; + } + + size_t copy_to_array(T* array[], size_t array_length) const { + size_t sz = 0; + for (LinkedListEntry* e = head_; sz < array_length && e != nullptr; e = e->next) { + array[sz++] = e->element; + } + + return sz; + } + + bool contains(const T* el) const { + for (LinkedListEntry* e = head_; e != nullptr; e = e->next) { + if (e->element == el) { return true; } } diff --git a/linker/linker.cpp b/linker/linker.cpp index 91ec49f90..3be2ec67a 100644 --- a/linker/linker.cpp +++ b/linker/linker.cpp @@ -609,6 +609,8 @@ done: return NULL; } + + // Another soinfo list allocator to use in dlsym. We don't reuse // SoinfoListAllocator because it is write-protected most of the time. static LinkerAllocator> g_soinfo_list_allocator_rw; @@ -861,10 +863,17 @@ static void soinfo_unload(soinfo* si) { si->CallDestructors(); if (si->has_min_version(0)) { - si->get_children().for_each([&] (soinfo* child) { - TRACE("%s needs to unload %s", si->name, child->name); - soinfo_unload(child); - }); + // It is not safe to do si->get_children().for_each, because + // during soinfo_free the child will concurrently modify the si->children + // list, therefore we create a copy and use it to unload children. + size_t children_count = si->get_children().size(); + soinfo* children[children_count]; + si->get_children().copy_to_array(children, children_count); + + for (size_t i = 0; i < children_count; ++i) { + TRACE("%s needs to unload %s", si->name, children[i]->name); + soinfo_unload(children[i]); + } } else { for (ElfW(Dyn)* d = si->dynamic; d->d_tag != DT_NULL; ++d) { if (d->d_tag == DT_NEEDED) { @@ -1618,7 +1627,7 @@ void soinfo::remove_all_links() { }); parents.for_each([&] (soinfo* parent) { - parent->children.for_each([&] (const soinfo* child) { + parent->children.remove_if([&] (const soinfo* child) { return child == this; }); }); diff --git a/linker/tests/linked_list_test.cpp b/linker/tests/linked_list_test.cpp index b9816fa10..0483b8412 100644 --- a/linker/tests/linked_list_test.cpp +++ b/linker/tests/linked_list_test.cpp @@ -80,7 +80,7 @@ TEST(linked_list, simple) { }); ASSERT_TRUE(!alloc_called); - ASSERT_TRUE(!free_called); + ASSERT_TRUE(free_called); ASSERT_EQ("dba", test_list_to_string(list)); alloc_called = free_called = false; @@ -103,15 +103,82 @@ TEST(linked_list, push_pop) { ASSERT_EQ("ab", test_list_to_string(list)); list.push_back("c"); ASSERT_EQ("abc", test_list_to_string(list)); - ASSERT_EQ("a", list.pop_front()); + ASSERT_STREQ("a", list.pop_front()); ASSERT_EQ("bc", test_list_to_string(list)); - ASSERT_EQ("b", list.pop_front()); + ASSERT_STREQ("b", list.pop_front()); ASSERT_EQ("c", test_list_to_string(list)); - ASSERT_EQ("c", list.pop_front()); + ASSERT_STREQ("c", list.pop_front()); ASSERT_EQ("", test_list_to_string(list)); ASSERT_TRUE(list.pop_front() == nullptr); list.push_back("r"); ASSERT_EQ("r", test_list_to_string(list)); - ASSERT_EQ("r", list.pop_front()); + ASSERT_STREQ("r", list.pop_front()); ASSERT_TRUE(list.pop_front() == nullptr); } + +TEST(linked_list, remove_if_then_pop) { + test_list_t list; + list.push_back("a"); + list.push_back("b"); + list.push_back("c"); + list.push_back("d"); + list.remove_if([](const char* c) { + return *c == 'b' || *c == 'c'; + }); + + ASSERT_EQ("ad", test_list_to_string(list)); + ASSERT_STREQ("a", list.pop_front()); + ASSERT_EQ("d", test_list_to_string(list)); + ASSERT_STREQ("d", list.pop_front()); + ASSERT_TRUE(list.pop_front() == nullptr); +} + +TEST(linked_list, copy_to_array) { + test_list_t list; + const size_t max_size = 128; + const char* buf[max_size]; + memset(buf, 0, sizeof(buf)); + + ASSERT_EQ(0U, list.size()); + ASSERT_EQ(0U, list.copy_to_array(buf, max_size)); + ASSERT_EQ(nullptr, buf[0]); + + list.push_back("a"); + list.push_back("b"); + list.push_back("c"); + list.push_back("d"); + + memset(buf, 0, sizeof(buf)); + ASSERT_EQ(4U, list.size()); + ASSERT_EQ(2U, list.copy_to_array(buf, 2)); + ASSERT_EQ('a', *buf[0]); + ASSERT_EQ('b', *buf[1]); + ASSERT_EQ(nullptr, buf[2]); + + ASSERT_EQ(4U, list.copy_to_array(buf, max_size)); + ASSERT_EQ('a', *buf[0]); + ASSERT_EQ('b', *buf[1]); + ASSERT_EQ('c', *buf[2]); + ASSERT_EQ('d', *buf[3]); + ASSERT_EQ(nullptr, buf[4]); + + memset(buf, 0, sizeof(buf)); + list.remove_if([](const char* c) { + return *c != 'c'; + }); + ASSERT_EQ(1U, list.size()); + ASSERT_EQ(1U, list.copy_to_array(buf, max_size)); + ASSERT_EQ('c', *buf[0]); + ASSERT_EQ(nullptr, buf[1]); + + memset(buf, 0, sizeof(buf)); + + list.remove_if([](const char* c) { + return *c == 'c'; + }); + + ASSERT_EQ(0U, list.size()); + ASSERT_EQ(0U, list.copy_to_array(buf, max_size)); + ASSERT_EQ(nullptr, buf[0]); +} +