We recently uncovered a memory leak that we were encountering, and discovered it was due to Eigen's resize mechanisms:
https://github.com/RobotLocomotion/drake/issues/5974

Specifically, we were storing symbolic variables which directly contained std::string variables. This was a problem if the strings were short enough for SSO, and would cause an unsafe memcpy-move, since the internal data was not invariant of the storage location (storing an offset into this).

Specifically, in PlainObjectBase.h, under conservative_resize_like_impl, we encountered a snag here:

Code: Select all

template <typename Derived, typename OtherDerived, bool IsVector>
struct conservative_resize_like_impl
{
  // eacousineau: Always called via: VectorXd::conservativeResize(int, int)
  static void run(DenseBase<Derived>& _this, Index rows, Index cols)
  {
    if (_this.rows() == rows && _this.cols() == cols) return;
    EIGEN_STATIC_ASSERT_DYNAMIC_SIZE(Derived)

    if ( ... /* eacousineau: We are using ColMajor */
         (!Derived::IsRowMajor && _this.rows() == rows) )  // column-major and we change only the number of columns
    {
      // ...
      // eacousineau: This *would* make us do a potentially unsafe move.
      //   However, this code path is never executed for an actual vector
      _this.derived().m_storage.conservativeResize(rows*cols,rows,cols);
    }
    else
    {
      // The storage order does not allow us to use reallocation.
      typename Derived::PlainObject tmp(rows,cols);
      // ...
      tmp.block(0,0,common_rows,common_cols) = _this.block(0,0,common_rows,common_cols);
      // eacousineau: This permits us to do a safe move
      //   (though inefficient, due to copying, if the compiler does no elision)
      _this.derived().swap(tmp);
    }
  }
...
};

template <typename Derived, typename OtherDerived>
struct conservative_resize_like_impl<Derived,OtherDerived,true>
  : conservative_resize_like_impl<Derived,OtherDerived,false>
{
  // eacousineau: This overload permits the above-mentioned codepaths
  using conservative_resize_like_impl<Derived,OtherDerived,false>::run;
 
  // eacousineau: Always called via: VectorXd::conservativeResize(int)
  static void run(DenseBase<Derived>& _this, Index size)
  {
    // ...
    // eacousineau: This is always unsafe
    _this.derived().m_storage.conservativeResize(size,new_rows,new_cols);
  }
...
};


We have presently side-stepped the issue by making the Scalar type's std::string be stored in a std::shared_ptr<>, such that it is memcpy-movable:
https://github.com/RobotLocomotion/drak ... 540ece84ae
Utility for checking invariance:
https://github.com/RobotLocomotion/drak ... able.h#L21


I noticed that there is NumTraits<T>::RequireInitialization (Memory.h), and a method called smart_copy.

However, when looking at PlainObjectBase.h, I see:

Code: Select all

template <typename Derived, typename OtherDerived, bool IsVector>
struct conservative_resize_like_impl


which, I think (have not yet fully traced it out), ends up calling something like this (Memory.h):

Code: Select all

conditional_aligned_realloc_new_auto<...>
    // Goes to:
        conditional_aligned_realloc<...>


Question:

Is there something like NumTraits<T>::IsMemcpyMovable / RequireMoveConstructor, which would keep on the same codepaths (and would be true by default), but if it were false, it would explicitly move the objects?
(maybe with a C++11 guard, that actually causes the appropriate move constructor via std::move).

If not, do you have any suggestions for how this would be done?
And would this be considered a bug of sorts (or an enhancement), something that I should send to the mailing list?

Would it also be possible to update the memory leak portion of the Wiki to indicate this?

Sorry for late reply,

I guess c++11 exposes all the required type traits to automatically fix this issue on Eigen's side, leaving it as a "known pitfall" in C++98. Would you be able to propose a patch?

I added an entry: http://eigen.tuxfamily.org/bz/show_bug.cgi?id=1432