hcq/MaxScale
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix pointer manipulation in LRUStorage

Browse Source 
The head and tail pointers were not properly updated in
all circumstances.
This commit is contained in:
Johan Wikman
2016-12-13 22:23:36 +02:00
parent 9f08e1300f
commit a8a5bafdb6
2 changed files with 117 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files

166
server/modules/filter/cache/lrustorage.cc vendored
View File

@ -78,12 +78,7 @@ cache_result_t LRUStorage::do_get_value(const CACHE_KEY& key,
if (existed)
{
if (ptail_ == i->second)
{
ptail_ = i->second->prev();
}
phead_ = i->second->prepend(phead_);
move_to_head(i->second);
}
else
{
@ -98,23 +93,8 @@ cache_result_t LRUStorage::do_get_value(const CACHE_KEY& key,
if (existed && (result == CACHE_RESULT_NOT_FOUND))
{
// We'll assume this is because ttl has hit in. We need to remove
// the mapping and the node.
Node* pnode = i->second;
if (phead_ == pnode)
{
phead_ = pnode->next();
}
if (ptail_ == pnode)
{
ptail_ = pnode->prev();
}
delete pnode;
nodes_per_key_.erase(i);
// the node and the mapping.
free_node(i);
}
}
@ -152,7 +132,6 @@ cache_result_t LRUStorage::do_put_value(const CACHE_KEY& key, const GWBUF* pvalu
else
{
ss_dassert(stats_.items == max_count_);
MXS_NOTICE("Max count %lu reached, removing least recently used.", max_count_);
pnode = free_lru();
}
}
@ -200,23 +179,12 @@ cache_result_t LRUStorage::do_put_value(const CACHE_KEY& key, const GWBUF* pvalu
pnode->reset(&i->first, value_size);
stats_.size += pnode->size();
if (ptail_ == pnode)
{
ptail_ = pnode->prev();
}
phead_ = pnode->prepend(phead_);
if (!ptail_)
{
ptail_ = phead_;
}
move_to_head(pnode);
}
else if (!existed)
{
MXS_ERROR("Could not put a value to the storage.");
nodes_per_key_.erase(i);
delete pnode;
free_node(i);
}
}
@ -226,40 +194,33 @@ cache_result_t LRUStorage::do_put_value(const CACHE_KEY& key, const GWBUF* pvalu
cache_result_t LRUStorage::do_del_value(const CACHE_KEY& key)
{
NodesPerKey::iterator i = nodes_per_key_.find(key);
bool existed = (i != nodes_per_key_.end());
cache_result_t result = pstorage_->del_value(key);
if (result == CACHE_RESULT_OK)
{
++stats_.deletes;
if (i == nodes_per_key_.end())
{
Node* pnode = i->second;
ss_dassert(stats_.size > pnode->size());
ss_dassert(stats_.items > 0);
stats_.size -= pnode->size();
--stats_.items;
phead_ = pnode->remove();
delete pnode;
if (!phead_)
{
ptail_ = NULL;
}
nodes_per_key_.erase(i);
}
else
if (!existed)
{
ss_dassert(!true);
MXS_ERROR("Key was found from storage, but not from LRU register.");
}
}
if (existed && ((result == CACHE_RESULT_OK) || (result == CACHE_RESULT_NOT_FOUND)))
{
// If it wasn't found, we'll assume it was because ttl has hit in.
++stats_.deletes;
ss_dassert(stats_.size >= i->second->size());
ss_dassert(stats_.items > 0);
stats_.size -= i->second->size();
--stats_.items;
free_node(i);
}
return result;
}
@ -328,7 +289,8 @@ LRUStorage::Node* LRUStorage::free_lru()
if (free_node_data(ptail_))
{
pnode = ptail_;
ptail_ = ptail_->remove();
remove_node(pnode);
}
return pnode;
@ -356,7 +318,8 @@ LRUStorage::Node* LRUStorage::free_lru(size_t needed_space)
freed_space += size;
pnode = ptail_;
ptail_ = ptail_->remove();
remove_node(pnode);
if (freed_space < needed_space)
{
@ -429,6 +392,85 @@ bool LRUStorage::free_node_data(Node* pnode)
return success;
}
/**
* Free a node and update head/tail accordingly.
*
* @param pnode The node to be freed.
*/
void LRUStorage::free_node(Node* pnode) const
{
remove_node(pnode);
delete pnode;
ss_dassert(!phead_ || (phead_->prev() == NULL));
ss_dassert(!ptail_ || (ptail_->next() == NULL));
}
/**
* Free the node referred to by the iterator and update head/tail accordingly.
*
* @param i The map iterator.
*/
void LRUStorage::free_node(NodesPerKey::iterator& i) const
{
free_node(i->second); // A Node
nodes_per_key_.erase(i);
}
/**
* Remove a node and update head/tail accordingly.
*
* @param pnode The node to be removed.
*/
void LRUStorage::remove_node(Node* pnode) const
{
ss_dassert(phead_->prev() == NULL);
ss_dassert(ptail_->next() == NULL);
if (phead_ == pnode)
{
phead_ = phead_->next();
}
if (ptail_ == pnode)
{
ptail_ = ptail_->prev();
}
pnode->remove();
ss_dassert(!phead_ || (phead_->prev() == NULL));
ss_dassert(!ptail_ || (ptail_->next() == NULL));
}
/**
* Move a node to head.
*
* @param pnode The node to be moved to head.
*/
void LRUStorage::move_to_head(Node* pnode) const
{
ss_dassert(!phead_ || (phead_->prev() == NULL));
ss_dassert(!ptail_ || (ptail_->next() == NULL));
if (ptail_ == pnode)
{
ptail_ = pnode->prev();
}
phead_ = pnode->prepend(phead_);
if (!ptail_)
{
ptail_ = phead_;
}
ss_dassert(phead_);
ss_dassert(ptail_);
ss_dassert((phead_ != ptail_) || (phead_ == pnode));
ss_dassert(phead_->prev() == NULL);
ss_dassert(ptail_->next() == NULL);
}
static void set_integer(json_t* pobject, const char* zname, size_t value)
{

17
server/modules/filter/cache/lrustorage.hh vendored
View File

@ -112,7 +112,7 @@ private:
*/
Node* prepend(Node* pnode)
{
if (pnode)
if (pnode && (pnode != this))
{
if (pprev_)
{
@ -155,7 +155,12 @@ private:
pnext_->pprev_ = pprev_;
}
return pprev_ ? pprev_ : pnext_;
Node* pnode = (pprev_ ? pprev_ : pnext_);
pprev_ = NULL;
pnext_ = NULL;
return pnode;
}
void reset(const CACHE_KEY* pkey = NULL, size_t size = 0)
@ -171,13 +176,17 @@ private:
Node* pprev_; /*< The previous node in the LRU list. */
};
typedef std::tr1::unordered_map<CACHE_KEY, Node*> NodesPerKey;
Node* free_lru();
Node* free_lru(size_t space);
bool free_node_data(Node* pnode);
void free_node(Node* pnode) const;
void free_node(NodesPerKey::iterator& i) const;
void remove_node(Node* pnode) const;
void move_to_head(Node* pnode) const;
private:
typedef std::tr1::unordered_map<CACHE_KEY, Node*> NodesPerKey;
struct Stats
{
Stats()