From cf7c1afb9354914224c2afa1d55ae7362ef3f459 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 14 Mar 2016 08:04:47 +0100 Subject: [PATCH 1/4] tag/TagPool: use prime number for NUM_SLOTS --- src/tag/TagPool.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tag/TagPool.cxx b/src/tag/TagPool.cxx index 29f605337..dc8cc5551 100644 --- a/src/tag/TagPool.cxx +++ b/src/tag/TagPool.cxx @@ -29,7 +29,7 @@ Mutex tag_pool_lock; -static constexpr size_t NUM_SLOTS = 4096; +static constexpr size_t NUM_SLOTS = 4093; struct TagPoolSlot { TagPoolSlot *next; From f1285a6dfd7abac5e2151040d9edbc70ed4e9dba Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 14 Mar 2016 08:07:22 +0100 Subject: [PATCH 2/4] tag/TagPool: add constexpr MAX_REF --- src/tag/TagPool.cxx | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/tag/TagPool.cxx b/src/tag/TagPool.cxx index dc8cc5551..55655cb54 100644 --- a/src/tag/TagPool.cxx +++ b/src/tag/TagPool.cxx @@ -23,6 +23,8 @@ #include "util/Cast.hxx" #include "util/VarSize.hxx" +#include + #include #include #include @@ -36,6 +38,8 @@ struct TagPoolSlot { unsigned char ref; TagItem item; + static constexpr unsigned MAX_REF = std::numeric_limits::max(); + TagPoolSlot(TagPoolSlot *_next, TagType type, const char *value, size_t length) :next(_next), ref(1) { @@ -116,7 +120,7 @@ tag_pool_get_item(TagType type, const char *value, size_t length) if (slot->item.type == type && length == strlen(slot->item.value) && memcmp(value, slot->item.value, length) == 0 && - slot->ref < 0xff) { + slot->ref < TagPoolSlot::MAX_REF) { assert(slot->ref > 0); ++slot->ref; return &slot->item; @@ -135,11 +139,11 @@ tag_pool_dup_item(TagItem *item) assert(slot->ref > 0); - if (slot->ref < 0xff) { + if (slot->ref < TagPoolSlot::MAX_REF) { ++slot->ref; return item; } else { - /* the reference counter overflows above 0xff; + /* the reference counter overflows above MAX_REF; duplicate the item, and start with 1 */ size_t length = strlen(item->value); auto slot_p = tag_value_slot_p(item->type, From a3afd5178c249a7799881598379b9230ae75616a Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 14 Mar 2016 13:08:04 +0100 Subject: [PATCH 3/4] tag/TagPool: optimize _dup_item() When a reference counter is at its limit, don't allocate a new TagPoolSlot - that would result in many TagPoolSlot instances with ref==1. This in turn would make the linked list very very large, which means quadratic runtime for many operations. --- NEWS | 1 + src/tag/TagPool.cxx | 10 +++------- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/NEWS b/NEWS index 62ea0c08e..120557636 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,7 @@ ver 0.19.14 (not yet released) - opus: limit tag size to 64 kB * archive - iso9660: fix buffer overflow +* fix quadratic runtime bug in the tag pool * fix build failures on non-glibc builds due to constexpr Mutex ver 0.19.13 (2016/02/23) diff --git a/src/tag/TagPool.cxx b/src/tag/TagPool.cxx index 55655cb54..97015b497 100644 --- a/src/tag/TagPool.cxx +++ b/src/tag/TagPool.cxx @@ -144,14 +144,10 @@ tag_pool_dup_item(TagItem *item) return item; } else { /* the reference counter overflows above MAX_REF; - duplicate the item, and start with 1 */ + obtain a reference to a different TagPoolSlot which + isn't yet "full" */ size_t length = strlen(item->value); - auto slot_p = tag_value_slot_p(item->type, - item->value, length); - slot = TagPoolSlot::Create(*slot_p, item->type, - item->value, strlen(item->value)); - *slot_p = slot; - return &slot->item; + return tag_pool_get_item(item->type, item->value, length); } } From ff35aa07dc5557361b9f10649a73c436bcdbb375 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 18 Mar 2016 18:26:58 +0100 Subject: [PATCH 4/4] release v0.19.14 --- NEWS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 120557636..0762ce40e 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,4 @@ -ver 0.19.14 (not yet released) +ver 0.19.14 (2016/03/18) * decoder - dsdiff: fix off-by-one buffer overflow - opus: limit tag size to 64 kB