Skip to content

TS-4629: Fix use of invalid iterator in ServerSessionPool::purge. #780

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
SolidWallOfCode wants to merge 1 commit into from
Closed

TS-4629: Fix use of invalid iterator in ServerSessionPool::purge. #780

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion lib/ts/Map.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1306,14 +1306,15 @@ class TSHashTable

/** Standard iterator for walking the table.
This iterates over all elements.
@internal Iterator is end if m_value is NULL.
@internal Iterator is @a end if @a m_value is @c NULL.
*/
struct iterator {
Value *m_value; ///< Current location.
Bucket *m_bucket; ///< Current bucket;

iterator() : m_value(0), m_bucket(0) {}
iterator &operator++();
iterator operator++(int);
Value &operator*() { return *m_value; }
Value *operator->() { return m_value; }
bool
Expand Down Expand Up @@ -1495,6 +1496,13 @@ template <typename H> typename TSHashTable<H>::iterator &TSHashTable<H>::iterato
return *this;
}

template <typename H> typename TSHashTable<H>::iterator TSHashTable<H>::iterator::operator++(int)
{
iterator prev(*this);
++*this;
return prev;
}

template <typename H>
TSHashTable<H>::TSHashTable(size_t nb) : m_count(0), m_expansion_policy(AVERAGE), m_expansion_limit(DEFAULT_EXPANSION_LIMIT)
{
Expand Down
7 changes: 4 additions & 3 deletions proxy/http/HttpSessionManager.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,10 @@ ServerSessionPool::ServerSessionPool() : Continuation(new_ProxyMutex()), m_ip_po
void
ServerSessionPool::purge()
{
for (IPHashTable::iterator last = m_ip_pool.end(), spot = m_ip_pool.begin(); spot != last; ++spot) {
spot->do_io_close();
}
// @c do_io_close can free the instance which clears the intrusive links and breaks the iterator.
// Therefore @c do_io_close is called on a post-incremented iterator.
for (IPHashTable::iterator last = m_ip_pool.end(), spot = m_ip_pool.begin(); spot != last ; spot++->do_io_close())
; // empty
m_ip_pool.clear();
Copy link
Contributor

@dragon512 dragon512 Jul 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a thought. As at this point the collection should be empty? do want to add an assert that it is vs calling clear() as calling clear() on an empty collection be a no-opt or a sign of a bug?

m_host_pool.clear();
}
Expand Down