From c3342b0bb4e5c9b48a0867f4a3ac4f909bbf0c91 Mon Sep 17 00:00:00 2001 From: Gael Guennebaud Date: Sat, 19 Mar 2011 01:06:50 +0100 Subject: [PATCH] fix memory leak when a custom scalar throw an exception (transplanted from 290205dfc049abc5a92c1191740b30fa91130ade ) --- Eigen/src/Core/Product.h | 35 ++------ Eigen/src/Core/SolveTriangular.h | 17 ++-- Eigen/src/Core/products/GeneralMatrixMatrix.h | 16 ++-- .../products/GeneralMatrixMatrixTriangular.h | 6 +- .../Core/products/SelfadjointMatrixMatrix.h | 15 +--- .../Core/products/SelfadjointMatrixVector.h | 41 +++------ Eigen/src/Core/products/SelfadjointProduct.h | 20 ++--- .../Core/products/TriangularMatrixMatrix.h | 16 +--- .../Core/products/TriangularMatrixVector.h | 43 ++------- .../Core/products/TriangularSolverMatrix.h | 14 +-- Eigen/src/Core/util/Memory.h | 90 +++++++++++++------ 11 files changed, 120 insertions(+), 193 deletions(-) diff --git a/Eigen/src/Core/Product.h b/Eigen/src/Core/Product.h index 1363e83b6..bde25375d 100644 --- a/Eigen/src/Core/Product.h +++ b/Eigen/src/Core/Product.h @@ -416,23 +416,15 @@ template<> struct gemv_selector RhsScalar compatibleAlpha = get_factor::run(actualAlpha); - ResScalar* actualDestPtr; - bool freeDestPtr = false; - if (evalToDest) - { - actualDestPtr = &dest.coeffRef(0); - } - else + ei_declare_aligned_stack_constructed_variable(ResScalar,actualDestPtr,dest.size(), + evalToDest ? dest.data() : static_dest.data()); + + if(!evalToDest) { #ifdef EIGEN_DENSE_STORAGE_CTOR_PLUGIN int size = dest.size(); EIGEN_DENSE_STORAGE_CTOR_PLUGIN #endif - if((actualDestPtr = static_dest.data())==0) - { - freeDestPtr = true; - actualDestPtr = ei_aligned_stack_new(ResScalar,dest.size()); - } if(!alphaIsCompatible) { MappedDest(actualDestPtr, dest.size()).setZero(); @@ -456,7 +448,6 @@ template<> struct gemv_selector dest += actualAlpha * MappedDest(actualDestPtr, dest.size()); else dest = MappedDest(actualDestPtr, dest.size()); - if(freeDestPtr) ei_aligned_stack_delete(ResScalar, actualDestPtr, dest.size()); } } }; @@ -490,23 +481,15 @@ template<> struct gemv_selector gemv_static_vector_if static_rhs; - RhsScalar* actualRhsPtr; - bool freeRhsPtr = false; - if (DirectlyUseRhs) - { - actualRhsPtr = const_cast(&actualRhs.coeffRef(0)); - } - else + ei_declare_aligned_stack_constructed_variable(RhsScalar,actualRhsPtr,actualRhs.size(), + DirectlyUseRhs ? const_cast(actualRhs.data()) : static_rhs.data()); + + if(!DirectlyUseRhs) { #ifdef EIGEN_DENSE_STORAGE_CTOR_PLUGIN int size = actualRhs.size(); EIGEN_DENSE_STORAGE_CTOR_PLUGIN #endif - if((actualRhsPtr = static_rhs.data())==0) - { - freeRhsPtr = true; - actualRhsPtr = ei_aligned_stack_new(RhsScalar, actualRhs.size()); - } Map(actualRhsPtr, actualRhs.size()) = actualRhs; } @@ -517,8 +500,6 @@ template<> struct gemv_selector actualRhsPtr, 1, &dest.coeffRef(0,0), dest.innerStride(), actualAlpha); - - if((!DirectlyUseRhs) && freeRhsPtr) ei_aligned_stack_delete(RhsScalar, actualRhsPtr, prod.rhs().size()); } }; diff --git a/Eigen/src/Core/SolveTriangular.h b/Eigen/src/Core/SolveTriangular.h index 7cbcf3d80..71e129c7f 100644 --- a/Eigen/src/Core/SolveTriangular.h +++ b/Eigen/src/Core/SolveTriangular.h @@ -74,26 +74,19 @@ struct triangular_solver_selector // FIXME find a way to allow an inner stride if packet_traits::size==1 bool useRhsDirectly = Rhs::InnerStrideAtCompileTime==1 || rhs.innerStride()==1; - RhsScalar* actualRhs; - if(useRhsDirectly) - { - actualRhs = &rhs.coeffRef(0); - } - else - { - actualRhs = ei_aligned_stack_new(RhsScalar,rhs.size()); + + ei_declare_aligned_stack_constructed_variable(RhsScalar,actualRhs,rhs.size(), + (useRhsDirectly ? rhs.data() : 0)); + + if(!useRhsDirectly) MappedRhs(actualRhs,rhs.size()) = rhs; - } triangular_solve_vector ::run(actualLhs.cols(), actualLhs.data(), actualLhs.outerStride(), actualRhs); if(!useRhsDirectly) - { rhs = MappedRhs(actualRhs, rhs.size()); - ei_aligned_stack_delete(RhsScalar, actualRhs, rhs.size()); - } } }; diff --git a/Eigen/src/Core/products/GeneralMatrixMatrix.h b/Eigen/src/Core/products/GeneralMatrixMatrix.h index 7736a4b29..d558b4759 100644 --- a/Eigen/src/Core/products/GeneralMatrixMatrix.h +++ b/Eigen/src/Core/products/GeneralMatrixMatrix.h @@ -94,8 +94,9 @@ static void run(Index rows, Index cols, Index depth, std::size_t sizeA = kc*mc; std::size_t sizeW = kc*Traits::WorkSpaceFactor; - LhsScalar* blockA = ei_aligned_stack_new(LhsScalar, sizeA); - RhsScalar* w = ei_aligned_stack_new(RhsScalar, sizeW); + ei_declare_aligned_stack_constructed_variable(LhsScalar, blockA, sizeA, 0); + ei_declare_aligned_stack_constructed_variable(RhsScalar, w, sizeW, 0); + RhsScalar* blockB = blocking.blockB(); eigen_internal_assert(blockB!=0); @@ -167,9 +168,10 @@ static void run(Index rows, Index cols, Index depth, std::size_t sizeA = kc*mc; std::size_t sizeB = kc*cols; std::size_t sizeW = kc*Traits::WorkSpaceFactor; - LhsScalar *blockA = blocking.blockA()==0 ? ei_aligned_stack_new(LhsScalar, sizeA) : blocking.blockA(); - RhsScalar *blockB = blocking.blockB()==0 ? ei_aligned_stack_new(RhsScalar, sizeB) : blocking.blockB(); - RhsScalar *blockW = blocking.blockW()==0 ? ei_aligned_stack_new(RhsScalar, sizeW) : blocking.blockW(); + + ei_declare_aligned_stack_constructed_variable(LhsScalar, blockA, sizeA, blocking.blockA()); + ei_declare_aligned_stack_constructed_variable(RhsScalar, blockB, sizeB, blocking.blockB()); + ei_declare_aligned_stack_constructed_variable(RhsScalar, blockW, sizeW, blocking.blockW()); // For each horizontal panel of the rhs, and corresponding panel of the lhs... // (==GEMM_VAR1) @@ -200,10 +202,6 @@ static void run(Index rows, Index cols, Index depth, } } - - if(blocking.blockA()==0) ei_aligned_stack_delete(LhsScalar, blockA, sizeA); - if(blocking.blockB()==0) ei_aligned_stack_delete(RhsScalar, blockB, sizeB); - if(blocking.blockW()==0) ei_aligned_stack_delete(RhsScalar, blockW, sizeW); } } diff --git a/Eigen/src/Core/products/GeneralMatrixMatrixTriangular.h b/Eigen/src/Core/products/GeneralMatrixMatrixTriangular.h index 39495c1a2..562c9c5ad 100644 --- a/Eigen/src/Core/products/GeneralMatrixMatrixTriangular.h +++ b/Eigen/src/Core/products/GeneralMatrixMatrixTriangular.h @@ -83,10 +83,10 @@ struct general_matrix_matrix_triangular_product Traits::nr) mc = (mc/Traits::nr)*Traits::nr; - LhsScalar* blockA = ei_aligned_stack_new(LhsScalar, kc*mc); std::size_t sizeW = kc*Traits::WorkSpaceFactor; std::size_t sizeB = sizeW + kc*size; - RhsScalar* allocatedBlockB = ei_aligned_stack_new(RhsScalar, sizeB); + ei_declare_aligned_stack_constructed_variable(LhsScalar, blockA, kc*mc, 0); + ei_declare_aligned_stack_constructed_variable(RhsScalar, allocatedBlockB, sizeB, 0); RhsScalar* blockB = allocatedBlockB + sizeW; gemm_pack_lhs pack_lhs; @@ -125,8 +125,6 @@ struct general_matrix_matrix_triangular_product gebp_kernel; @@ -313,9 +313,6 @@ struct product_selfadjoint_matrix(kc, mc, nc); - - Scalar* blockA = ei_aligned_stack_new(Scalar, kc*mc); std::size_t sizeW = kc*Traits::WorkSpaceFactor; std::size_t sizeB = sizeW + kc*cols; - Scalar* allocatedBlockB = ei_aligned_stack_new(Scalar, sizeB); + ei_declare_aligned_stack_constructed_variable(Scalar, blockA, kc*mc, 0); + ei_declare_aligned_stack_constructed_variable(Scalar, allocatedBlockB, sizeB, 0); Scalar* blockB = allocatedBlockB + sizeW; gebp_kernel gebp_kernel; @@ -369,9 +365,6 @@ struct product_selfadjoint_matrix(_rhs) : 0); if (rhsIncr!=1) { - Scalar* r = ei_aligned_stack_new(Scalar, size); const Scalar* it = _rhs; for (Index i=0; i(rhs), size); } } // end namespace internal @@ -211,40 +206,28 @@ struct SelfadjointProductMatrix internal::gemv_static_vector_if static_dest; internal::gemv_static_vector_if static_rhs; + + ei_declare_aligned_stack_constructed_variable(ResScalar,actualDestPtr,dest.size(), + EvalToDest ? dest.data() : static_dest.data()); + + ei_declare_aligned_stack_constructed_variable(RhsScalar,actualRhsPtr,rhs.size(), + UseRhs ? const_cast(rhs.data()) : static_rhs.data()); - bool freeDestPtr = false; - ResScalar* actualDestPtr; - if(EvalToDest) - actualDestPtr = dest.data(); - else + if(!EvalToDest) { #ifdef EIGEN_DENSE_STORAGE_CTOR_PLUGIN int size = dest.size(); EIGEN_DENSE_STORAGE_CTOR_PLUGIN #endif - if((actualDestPtr=static_dest.data())==0) - { - freeDestPtr = true; - actualDestPtr = ei_aligned_stack_new(ResScalar,dest.size()); - } MappedDest(actualDestPtr, dest.size()) = dest; } - bool freeRhsPtr = false; - RhsScalar* actualRhsPtr; - if(UseRhs) - actualRhsPtr = const_cast(rhs.data()); - else + if(!UseRhs) { #ifdef EIGEN_DENSE_STORAGE_CTOR_PLUGIN int size = rhs.size(); EIGEN_DENSE_STORAGE_CTOR_PLUGIN #endif - if((actualRhsPtr=static_rhs.data())==0) - { - freeRhsPtr = true; - actualRhsPtr = ei_aligned_stack_new(RhsScalar,rhs.size()); - } Map(actualRhsPtr, rhs.size()) = rhs; } @@ -259,11 +242,7 @@ struct SelfadjointProductMatrix ); if(!EvalToDest) - { dest = MappedDest(actualDestPtr, dest.size()); - if(freeDestPtr) ei_aligned_stack_delete(ResScalar, actualDestPtr, dest.size()); - } - if(freeRhsPtr) ei_aligned_stack_delete(RhsScalar, actualRhsPtr, rhs.size()); } }; diff --git a/Eigen/src/Core/products/SelfadjointProduct.h b/Eigen/src/Core/products/SelfadjointProduct.h index da2e6ee20..3a4523fa4 100644 --- a/Eigen/src/Core/products/SelfadjointProduct.h +++ b/Eigen/src/Core/products/SelfadjointProduct.h @@ -81,27 +81,17 @@ struct selfadjoint_product_selector UseOtherDirectly = _ActualOtherType::InnerStrideAtCompileTime==1 }; internal::gemv_static_vector_if static_other; - - bool freeOtherPtr = false; - Scalar* actualOtherPtr; - if(UseOtherDirectly) - actualOtherPtr = const_cast(actualOther.data()); - else - { - if((actualOtherPtr=static_other.data())==0) - { - freeOtherPtr = true; - actualOtherPtr = ei_aligned_stack_new(Scalar,other.size()); - } + + ei_declare_aligned_stack_constructed_variable(Scalar, actualOtherPtr, other.size(), + (UseOtherDirectly ? const_cast(actualOther.data()) : static_other.data())); + + if(!UseOtherDirectly) Map(actualOtherPtr, actualOther.size()) = actualOther; - } selfadjoint_rank1_update::IsComplex, (!OtherBlasTraits::NeedToConjugate) && NumTraits::IsComplex> ::run(other.size(), mat.data(), mat.outerStride(), actualOtherPtr, actualAlpha); - - if((!UseOtherDirectly) && freeOtherPtr) ei_aligned_stack_delete(Scalar, actualOtherPtr, other.size()); } }; diff --git a/Eigen/src/Core/products/TriangularMatrixMatrix.h b/Eigen/src/Core/products/TriangularMatrixMatrix.h index 66a2515d0..8f7e58f62 100644 --- a/Eigen/src/Core/products/TriangularMatrixMatrix.h +++ b/Eigen/src/Core/products/TriangularMatrixMatrix.h @@ -118,11 +118,10 @@ struct product_triangular_matrix_matrix(kc, mc, nc); - - Scalar* blockA = ei_aligned_stack_new(Scalar, kc*mc); std::size_t sizeW = kc*Traits::WorkSpaceFactor; std::size_t sizeB = sizeW + kc*cols; - Scalar* allocatedBlockB = ei_aligned_stack_new(Scalar, sizeB); + ei_declare_aligned_stack_constructed_variable(Scalar, blockA, kc*mc, 0); + ei_declare_aligned_stack_constructed_variable(Scalar, allocatedBlockB, sizeB, 0); Scalar* blockB = allocatedBlockB + sizeW; Matrix triangularBuffer; @@ -208,10 +207,6 @@ struct product_triangular_matrix_matrix(kc, mc, nc); - Scalar* blockA = ei_aligned_stack_new(Scalar, kc*mc); std::size_t sizeW = kc*Traits::WorkSpaceFactor; std::size_t sizeB = sizeW + kc*cols; - Scalar* allocatedBlockB = ei_aligned_stack_new(Scalar,sizeB); + ei_declare_aligned_stack_constructed_variable(Scalar, blockA, kc*mc, 0); + ei_declare_aligned_stack_constructed_variable(Scalar, allocatedBlockB, sizeB, 0); Scalar* blockB = allocatedBlockB + sizeW; Matrix triangularBuffer; @@ -347,9 +342,6 @@ struct product_triangular_matrix_matrix, 0, OuterStride<> > LhsMap; @@ -95,9 +92,6 @@ struct product_triangular_matrix_vector, 0, OuterStride<> > LhsMap; @@ -185,7 +179,7 @@ struct TriangularProduct template void scaleAndAddTo(Dest& dst, Scalar alpha) const { eigen_assert(dst.rows()==m_lhs.rows() && dst.cols()==m_rhs.cols()); - + typedef TriangularProduct<(Mode & UnitDiag) | ((Mode & Lower) ? Upper : Lower),true,Transpose,false,Transpose,true> TriangularProductTranspose; Transpose dstT(dst); internal::trmv_selector<(int(internal::traits::Flags)&RowMajorBit) ? ColMajor : RowMajor>::run( @@ -235,23 +229,15 @@ template<> struct trmv_selector RhsScalar compatibleAlpha = get_factor::run(actualAlpha); - ResScalar* actualDestPtr; - bool freeDestPtr = false; - if (evalToDest) - { - actualDestPtr = dest.data(); - } - else + ei_declare_aligned_stack_constructed_variable(ResScalar,actualDestPtr,dest.size(), + evalToDest ? dest.data() : static_dest.data()); + + if(!evalToDest) { #ifdef EIGEN_DENSE_STORAGE_CTOR_PLUGIN int size = dest.size(); EIGEN_DENSE_STORAGE_CTOR_PLUGIN #endif - if((actualDestPtr = static_dest.data())==0) - { - freeDestPtr = true; - actualDestPtr = ei_aligned_stack_new(ResScalar,dest.size()); - } if(!alphaIsCompatible) { MappedDest(actualDestPtr, dest.size()).setZero(); @@ -277,7 +263,6 @@ template<> struct trmv_selector dest += actualAlpha * MappedDest(actualDestPtr, dest.size()); else dest = MappedDest(actualDestPtr, dest.size()); - if(freeDestPtr) ei_aligned_stack_delete(ResScalar, actualDestPtr, dest.size()); } } }; @@ -310,23 +295,15 @@ template<> struct trmv_selector gemv_static_vector_if static_rhs; - RhsScalar* actualRhsPtr; - bool freeRhsPtr = false; - if (DirectlyUseRhs) - { - actualRhsPtr = const_cast(actualRhs.data()); - } - else + ei_declare_aligned_stack_constructed_variable(RhsScalar,actualRhsPtr,actualRhs.size(), + DirectlyUseRhs ? const_cast(actualRhs.data()) : static_rhs.data()); + + if(!DirectlyUseRhs) { #ifdef EIGEN_DENSE_STORAGE_CTOR_PLUGIN int size = actualRhs.size(); EIGEN_DENSE_STORAGE_CTOR_PLUGIN #endif - if((actualRhsPtr = static_rhs.data())==0) - { - freeRhsPtr = true; - actualRhsPtr = ei_aligned_stack_new(RhsScalar, actualRhs.size()); - } Map(actualRhsPtr, actualRhs.size()) = actualRhs; } @@ -340,8 +317,6 @@ template<> struct trmv_selector actualRhsPtr,1, dest.data(),dest.innerStride(), actualAlpha); - - if((!DirectlyUseRhs) && freeRhsPtr) ei_aligned_stack_delete(RhsScalar, actualRhsPtr, prod.rhs().size()); } }; diff --git a/Eigen/src/Core/products/TriangularSolverMatrix.h b/Eigen/src/Core/products/TriangularSolverMatrix.h index 8b9143c2b..a6ad9c322 100644 --- a/Eigen/src/Core/products/TriangularSolverMatrix.h +++ b/Eigen/src/Core/products/TriangularSolverMatrix.h @@ -70,10 +70,10 @@ struct triangular_solve_matrix(kc, mc, nc); - Scalar* blockA = ei_aligned_stack_new(Scalar, kc*mc); std::size_t sizeW = kc*Traits::WorkSpaceFactor; std::size_t sizeB = sizeW + kc*cols; - Scalar* allocatedBlockB = ei_aligned_stack_new(Scalar, sizeB); + ei_declare_aligned_stack_constructed_variable(Scalar, blockA, kc*mc, 0); + ei_declare_aligned_stack_constructed_variable(Scalar, allocatedBlockB, sizeB, 0); Scalar* blockB = allocatedBlockB + sizeW; conj_if conj; @@ -174,9 +174,6 @@ struct triangular_solve_matrix(kc, mc, nc); - Scalar* blockA = ei_aligned_stack_new(Scalar, kc*mc); std::size_t sizeW = kc*Traits::WorkSpaceFactor; std::size_t sizeB = sizeW + kc*size; - Scalar* allocatedBlockB = ei_aligned_stack_new(Scalar, sizeB); + ei_declare_aligned_stack_constructed_variable(Scalar, blockA, kc*mc, 0); + ei_declare_aligned_stack_constructed_variable(Scalar, allocatedBlockB, sizeB, 0); Scalar* blockB = allocatedBlockB + sizeW; conj_if conj; @@ -314,9 +311,6 @@ struct triangular_solve_matrixEIGEN_STACK_ALLOCATION_LIMIT) Eigen::internal::aligned_free(PTR) -#elif defined(_MSC_VER) - #define ei_aligned_stack_alloc(SIZE) (SIZE<=EIGEN_STACK_ALLOCATION_LIMIT) \ - ? _alloca(SIZE) \ - : Eigen::internal::aligned_malloc(SIZE) - #define ei_aligned_stack_free(PTR,SIZE) if(SIZE>EIGEN_STACK_ALLOCATION_LIMIT) Eigen::internal::aligned_free(PTR) -#else - #define ei_aligned_stack_alloc(SIZE) Eigen::internal::aligned_malloc(SIZE) - #define ei_aligned_stack_free(PTR,SIZE) Eigen::internal::aligned_free(PTR) +// you can overwrite Eigen's default behavior regarding alloca by defining EIGEN_ALLOCA +// to the appropriate stack allocation function +#ifndef EIGEN_ALLOCA + #if (defined __linux__) + #define EIGEN_ALLOCA alloca + #elif defined(_MSC_VER) + #define EIGEN_ALLOCA _alloca + #endif #endif -#define ei_aligned_stack_new(TYPE,SIZE) Eigen::internal::construct_elements_of_array(reinterpret_cast(ei_aligned_stack_alloc(sizeof(TYPE)*SIZE)), SIZE) -#define ei_aligned_stack_delete(TYPE,PTR,SIZE) do {Eigen::internal::destruct_elements_of_array(PTR, SIZE); \ - ei_aligned_stack_free(PTR,sizeof(TYPE)*SIZE);} while(0) +namespace internal { + +template class stack_memory_destructor +{ + public: + stack_memory_destructor(T* ptr,size_t size) : m_ptr(ptr), m_size(size) {} + ~stack_memory_destructor() + { + Eigen::internal::destruct_elements_of_array(m_ptr, m_size); + #ifdef EIGEN_ALLOCA + if(sizeof(T)*m_size>EIGEN_STACK_ALLOCATION_LIMIT) + #endif + Eigen::internal::aligned_free(m_ptr); + } + protected: + T* m_ptr; + size_t m_size; +}; + +} + +/** \internal + * Declares, allocates and construct an aligned buffer named NAME of SIZE elements of type TYPE on the stack + * if SIZE is smaller than EIGEN_STACK_ALLOCATION_LIMIT, and if stack allocation is supported by the platform + * (currently, this is Linux and Visual Studio only). Otherwise the memory is allocated on the heap. + * The allocated buffer is automatically deleted when exiting the scope of this declaration. + * If BUFFER is non nul, then the declared variable is simply an alias for BUFFER, and no allocation/deletion occurs. + * Here is an example: + * \code + * { + * ei_declare_aligned_stack_constructed_variable(float,data,size,0); + * // use data[0] to data[size-1] + * } + * \endcode + * The underlying stack allocation function can controlled with the EIGEN_ALLOCA preprocessor token. + */ +#ifdef EIGEN_ALLOCA + + #define ei_declare_aligned_stack_constructed_variable(TYPE,NAME,SIZE,BUFFER) \ + TYPE* NAME = (BUFFER)!=0 ? (BUFFER) \ + : reinterpret_cast( \ + (sizeof(TYPE)*SIZE<=EIGEN_STACK_ALLOCATION_LIMIT) ? alloca(sizeof(TYPE)*SIZE) \ + : Eigen::internal::aligned_malloc(sizeof(TYPE)*SIZE) ); \ + if((BUFFER)==0) Eigen::internal::construct_elements_of_array(NAME, SIZE); \ + Eigen::internal::stack_memory_destructor EIGEN_CAT(stack_memory_destructor,__LINE__)((BUFFER)==0 ? NAME : 0,SIZE) + +#else + + #define ei_declare_aligned_stack_constructed_variable(TYPE,NAME,SIZE,BUFFER) \ + TYPE* NAME = (BUFFER)!=0 ? BUFFER : reinterpret_cast(Eigen::internal::aligned_malloc(sizeof(TYPE)*SIZE)); \ + if((BUFFER)==0) Eigen::internal::construct_elements_of_array(NAME, SIZE); \ + Eigen::internal::stack_memory_destructor EIGEN_CAT(stack_memory_destructor,__LINE__)((BUFFER)==0 ? NAME : 0,SIZE) + +#endif /*****************************************************************************