From 7ed71e5324948bc79ea1e73ede06a2d46b275d06 Mon Sep 17 00:00:00 2001 From: Adam Wulkiewicz Date: Tue, 20 Nov 2012 17:00:41 +0000 Subject: [PATCH] Fixed memleak for CoordinateType/BoundingObject which have throwing copy constructor for R*tree. Added comments regarding exception-safety. [SVN r81438] --- .../extensions/index/nonassignable.hpp | 2 +- .../extensions/index/pushable_array.hpp | 2 + .../rtree/linear/redistribute_elements.hpp | 16 +++---- .../rtree/quadratic/redistribute_elements.hpp | 24 +++++----- .../rtree/rstar/redistribute_elements.hpp | 44 ++++++++++--------- .../index/rtree/visitors/insert.hpp | 42 +++++++++--------- 6 files changed, 68 insertions(+), 62 deletions(-) diff --git a/include/boost/geometry/extensions/index/nonassignable.hpp b/include/boost/geometry/extensions/index/nonassignable.hpp index c6ee9ebeb..810cfd111 100644 --- a/include/boost/geometry/extensions/index/nonassignable.hpp +++ b/include/boost/geometry/extensions/index/nonassignable.hpp @@ -1,6 +1,6 @@ // Boost.Geometry Index // -// Spatial query predicates +// Nonassignable base class. // // Copyright (c) 2011-2012 Adam Wulkiewicz, Lodz, Poland. // diff --git a/include/boost/geometry/extensions/index/pushable_array.hpp b/include/boost/geometry/extensions/index/pushable_array.hpp index 14570afdf..d01a82b81 100644 --- a/include/boost/geometry/extensions/index/pushable_array.hpp +++ b/include/boost/geometry/extensions/index/pushable_array.hpp @@ -133,6 +133,8 @@ public: m_size = 0; } + // IMPORTANT! + // The sequence of elements is NOT preserved inline void erase(iterator it) { BOOST_GEOMETRY_INDEX_ASSERT(0 < m_size, "there are no elements in the container"); diff --git a/include/boost/geometry/extensions/index/rtree/linear/redistribute_elements.hpp b/include/boost/geometry/extensions/index/rtree/linear/redistribute_elements.hpp index 6f4b237a8..910b77a39 100644 --- a/include/boost/geometry/extensions/index/rtree/linear/redistribute_elements.hpp +++ b/include/boost/geometry/extensions/index/rtree/linear/redistribute_elements.hpp @@ -224,7 +224,7 @@ struct redistribute_elements::apply(elements_copy, allocators); //elements_copy.clear(); - throw; // RETHROW + throw; // RETHROW, BASIC } } }; diff --git a/include/boost/geometry/extensions/index/rtree/quadratic/redistribute_elements.hpp b/include/boost/geometry/extensions/index/rtree/quadratic/redistribute_elements.hpp index b41ca6c61..97e5917ef 100644 --- a/include/boost/geometry/extensions/index/rtree/quadratic/redistribute_elements.hpp +++ b/include/boost/geometry/extensions/index/rtree/quadratic/redistribute_elements.hpp @@ -109,8 +109,8 @@ struct redistribute_elements::apply(elements_backup, allocators); //elements_backup.clear(); - throw; // RETHROW + throw; // RETHROW, BASIC } } diff --git a/include/boost/geometry/extensions/index/rtree/rstar/redistribute_elements.hpp b/include/boost/geometry/extensions/index/rtree/rstar/redistribute_elements.hpp index 469f1c702..7a81aa9f7 100644 --- a/include/boost/geometry/extensions/index/rtree/rstar/redistribute_elements.hpp +++ b/include/boost/geometry/extensions/index/rtree/rstar/redistribute_elements.hpp @@ -68,11 +68,11 @@ struct choose_split_axis_and_index_for_corner BOOST_GEOMETRY_INDEX_ASSERT(elements.size() == parameters.get_max_elements() + 1, "wrong number of elements"); // copy elements - Elements elements_copy(elements); // MAY THROW + Elements elements_copy(elements); // MAY THROW, STRONG (alloc, copy) // sort elements element_axis_corner_less elements_less(translator); - std::sort(elements_copy.begin(), elements_copy.end(), elements_less); // MAY THROW + std::sort(elements_copy.begin(), elements_copy.end(), elements_less); // MAY THROW, BASIC (copy) // init outputs choosen_index = parameters.get_min_elements(); @@ -135,7 +135,7 @@ struct choose_split_axis_and_index_for_axis choose_split_axis_and_index_for_corner:: apply(elements, index1, som1, ovl1, con1, - parameters, translator); // MAY THROW + parameters, translator); // MAY THROW, STRONG size_t index2 = 0; margin_type som2 = 0; @@ -145,7 +145,7 @@ struct choose_split_axis_and_index_for_axis choose_split_axis_and_index_for_corner:: apply(elements, index2, som2, ovl2, con2, - parameters, translator); // MAY THROW + parameters, translator); // MAY THROW, STRONG sum_of_margins = som1 + som2; @@ -185,7 +185,7 @@ struct choose_split_axis_and_index_for_axis:: apply(elements, choosen_index, sum_of_margins, smallest_overlap, smallest_content, - parameters, translator); // MAY THROW + parameters, translator); // MAY THROW, STRONG choosen_corner = min_corner; } @@ -215,7 +215,7 @@ struct choose_split_axis_and_index choose_split_axis_and_index:: apply(elements, choosen_axis, choosen_corner, choosen_index, smallest_sum_of_margins, smallest_overlap, smallest_content, - parameters, translator); // MAY THROW + parameters, translator); // MAY THROW, STRONG margin_type sum_of_margins = 0; @@ -230,7 +230,7 @@ struct choose_split_axis_and_index Box, Dimension - 1, typename index::traits::tag::type - >::apply(elements, corner, index, sum_of_margins, overlap_val, content_val, parameters, translator); // MAY THROW + >::apply(elements, corner, index, sum_of_margins, overlap_val, content_val, parameters, translator); // MAY THROW, STRONG if ( sum_of_margins < smallest_sum_of_margins ) { @@ -284,7 +284,7 @@ struct partial_sort { if ( axis < Dimension - 1 ) { - partial_sort::apply(elements, axis, index, tr); // MAY THROW + partial_sort::apply(elements, axis, index, tr); // MAY THROW, BASIC (copy) } else { @@ -292,7 +292,7 @@ struct partial_sort typedef typename Elements::value_type element_type; element_axis_corner_less less(tr); - std::partial_sort(elements.begin(), elements.begin() + index, elements.end(), less); // MAY THROW + std::partial_sort(elements.begin(), elements.begin() + index, elements.end(), less); // MAY THROW, BASIC (copy) } } }; @@ -307,7 +307,7 @@ struct partial_sort typedef typename Elements::value_type element_type; element_axis_corner_less less(tr); - std::partial_sort(elements.begin(), elements.begin() + index, elements.end(), less); // MAY THROW + std::partial_sort(elements.begin(), elements.begin() + index, elements.end(), less); // MAY THROW, BASIC (copy) } }; @@ -356,7 +356,7 @@ struct redistribute_elements::apply(elements1, split_axis, split_corner, split_index, smallest_sum_of_margins, smallest_overlap, smallest_content, - parameters, translator); // MAY THROW + parameters, translator); // MAY THROW, STRONG // TODO: awulkiew - get rid of following static_casts? @@ -365,23 +365,24 @@ struct redistribute_elements(min_corner) ) rstar::partial_sort - ::apply(elements_copy, split_axis, split_index, translator); // MAY THROW + ::apply(elements_copy, split_axis, split_index, translator); // MAY THROW, BASIC (copy) else rstar::partial_sort - ::apply(elements_copy, split_axis, split_index, translator); // MAY THROW + ::apply(elements_copy, split_axis, split_index, translator); // MAY THROW, BASIC (copy) try { // copy elements to nodes - elements1.resize(split_index); // MIGHT THROW - std::copy(elements_copy.begin(), elements_copy.begin() + split_index, elements1.begin()); // MAY THROW - elements2.resize(parameters.get_max_elements() + 1 - split_index); // MAY THROW - std::copy(elements_copy.begin() + split_index, elements_copy.end(), elements2.begin()); // MAY THROW + elements1.resize(split_index); // SHOULDN'T THROW + std::copy(elements_copy.begin(), elements_copy.begin() + split_index, elements1.begin()); // MAY THROW, BASIC + elements2.resize(parameters.get_max_elements() + 1 - split_index); // MAY THROW, STRONG + std::copy(elements_copy.begin() + split_index, elements_copy.end(), elements2.begin()); // MAY THROW, BASIC // calculate boxes box1 = rtree::elements_box(elements1.begin(), elements1.end(), translator); @@ -389,13 +390,14 @@ struct redistribute_elements::apply(elements_copy, allocators); - //elements_copy.clear(); + rtree::destroy_elements::apply(elements_backup, allocators); + //elements_backup.clear(); - throw; // RETHROW + throw; // RETHROW, BASIC } } }; diff --git a/include/boost/geometry/extensions/index/rtree/visitors/insert.hpp b/include/boost/geometry/extensions/index/rtree/visitors/insert.hpp index 9def4668a..36e051535 100644 --- a/include/boost/geometry/extensions/index/rtree/visitors/insert.hpp +++ b/include/boost/geometry/extensions/index/rtree/visitors/insert.hpp @@ -157,7 +157,7 @@ public: Box, Allocators, typename Options::redistribute_tag - >::apply(n, n2, n_box, box2, parameters, translator, allocators); // MAY THROW (V: alloc, copy, E: alloc) + >::apply(n, n2, n_box, box2, parameters, translator, allocators); // MAY THROW (V, E: alloc, copy, copy) // check numbers of elements BOOST_GEOMETRY_INDEX_ASSERT(parameters.get_min_elements() <= rtree::elements(n).size() && @@ -168,7 +168,7 @@ public: "unexpected number of elements"); // return the list of newly created nodes (this algorithm returns one) - additional_nodes.push_back(std::make_pair(box2, second_node.get())); // MAY THROW + additional_nodes.push_back(std::make_pair(box2, second_node.get())); // MAY THROW (alloc, copy) // release the ptr second_node.release(); @@ -269,7 +269,7 @@ protected: rtree::element_indexable(m_element, m_translator)); // next traversing step - traverse_apply_visitor(visitor, n, choosen_node_index); // MAY THROW (V: alloc, copy, E: alloc, N:alloc) + traverse_apply_visitor(visitor, n, choosen_node_index); // MAY THROW, BASIC (V, E: alloc, copy, N:alloc) } // TODO: awulkiew - change post_traverse name to handle_overflow or overflow_treatment? @@ -284,7 +284,7 @@ protected: // handle overflow if ( m_parameters.get_max_elements() < rtree::elements(n).size() ) { - split(n); // MAY THROW (V: alloc, copy, E: alloc, N:alloc) + split(n); // MAY THROW, BASIC (V, E: alloc, copy, N:alloc) } } @@ -298,7 +298,7 @@ protected: m_traverse_data.move_to_next_level(&n, choosen_node_index); // next traversing step - rtree::apply_visitor(visitor, *rtree::elements(n)[choosen_node_index].second); // MAY THROW (V: alloc, copy, E: alloc, N:alloc) + rtree::apply_visitor(visitor, *rtree::elements(n)[choosen_node_index].second); // MAY THROW, BASIC (V, E: alloc, copy, N:alloc) // restore previous traverse inputs m_traverse_data = backup_traverse_data; @@ -314,11 +314,12 @@ protected: typename split_algo::nodes_container_type additional_nodes; Box n_box; - split_algo::apply(additional_nodes, n, n_box, m_parameters, m_translator, m_allocators); // MAY THROW (V: alloc, copy, E: alloc, N:alloc) + split_algo::apply(additional_nodes, n, n_box, m_parameters, m_translator, m_allocators); // MAY THROW, BASIC (V, E: alloc, copy, N:alloc) BOOST_GEOMETRY_INDEX_ASSERT(additional_nodes.size() == 1, "unexpected number of additional nodes"); // TODO add all additional nodes + // For kmeans algorithm: // elements number may be greater than node max elements count // split and reinsert must take node with some elements count // and container of additional elements (std::pairs or Values) @@ -336,7 +337,7 @@ protected: // update old node's box m_traverse_data.current_element().first = n_box; // add new node to parent's children - m_traverse_data.parent_elements().push_back(additional_nodes[0]); // MAY THROW (V: alloc, copy, E: alloc) + m_traverse_data.parent_elements().push_back(additional_nodes[0]); // MAY THROW, STRONG (V, E: alloc, copy) } // node is the root - add level else @@ -344,15 +345,15 @@ protected: BOOST_GEOMETRY_INDEX_ASSERT(&n == rtree::get(m_root_node), "node should be the root"); // create new root and add nodes - node_auto_ptr new_root(rtree::create_node::apply(m_allocators), m_allocators); // MAY THROW (N:alloc) + node_auto_ptr new_root(rtree::create_node::apply(m_allocators), m_allocators); // MAY THROW, STRONG (N:alloc) try { - rtree::elements(rtree::get(*new_root)).push_back(std::make_pair(n_box, m_root_node)); // MAY THROW (E:alloc) - rtree::elements(rtree::get(*new_root)).push_back(additional_nodes[0]); // MAY THROW (E:alloc) + rtree::elements(rtree::get(*new_root)).push_back(std::make_pair(n_box, m_root_node)); // MAY THROW, STRONG (E:alloc, copy) + rtree::elements(rtree::get(*new_root)).push_back(additional_nodes[0]); // MAY THROW, STRONG (E:alloc, copy) } catch (...) { // clear new root to not delete in the ~node_auto_ptr() potentially stored old root node rtree::elements(rtree::get(*new_root)).clear(); - throw; // RETHROW + throw; // RETHROW, BASIC } m_root_node = new_root.get(); @@ -421,7 +422,7 @@ public: if ( base::m_traverse_data.current_level < base::m_level ) { // next traversing step - base::traverse(*this, n); // MAY THROW (E: alloc, N: alloc) + base::traverse(*this, n); // MAY THROW, BASIC (E: alloc, copy, N: alloc) } else { @@ -430,19 +431,20 @@ public: try { // push new child node - rtree::elements(n).push_back(base::m_element); // MAY THROW (E: alloc) + rtree::elements(n).push_back(base::m_element); // MAY THROW, STRONG (E: alloc, copy) } catch(...) { - // if the insert fails here, the element won't be stored in the tree + // if the insert fails above, the element won't be stored in the tree + rtree::visitors::destroy del_v(base::m_element.second, base::m_allocators); rtree::apply_visitor(del_v, *base::m_element.second); - throw; // RETHROW + throw; // RETHROW, BASIC } } - base::post_traverse(n); // MAY THROW (E: alloc, N: alloc) + base::post_traverse(n); // MAY THROW, BASIC (E: alloc, copy, N: alloc) } inline void operator()(leaf &) @@ -481,9 +483,9 @@ public: BOOST_GEOMETRY_INDEX_ASSERT(base::m_traverse_data.current_level < base::m_level, "unexpected level"); // next traversing step - base::traverse(*this, n); // MAY THROW (V: alloc, copy, E: alloc, N: alloc) + base::traverse(*this, n); // MAY THROW, BASIC (V, E: alloc, copy, N: alloc) - base::post_traverse(n); // MAY THROW (E: alloc, N: alloc) + base::post_traverse(n); // MAY THROW, BASIC (E: alloc, copy, N: alloc) } inline void operator()(leaf & n) @@ -492,9 +494,9 @@ public: BOOST_GEOMETRY_INDEX_ASSERT(base::m_level == base::m_traverse_data.current_level || base::m_level == (std::numeric_limits::max)(), "unexpected level"); - rtree::elements(n).push_back(base::m_element); // MAY THROW (V: alloc, copy) + rtree::elements(n).push_back(base::m_element); // MAY THROW, STRONG (V: alloc, copy) - base::post_traverse(n); // MAY THROW (V: alloc, copy, E: alloc, N: alloc) + base::post_traverse(n); // MAY THROW, BASIC (V: alloc, copy, N: alloc) } };