Opened 10 years ago

Last modified 10 years ago

#6844 new Bugs

[function] Memory leaks if used with allocator and throwing copy-c'tor of functor

Reported by: Daniel Krügler <daniel.kruegler@…> Owned by: Douglas Gregor
Milestone: To Be Determined Component: function
Version: Boost 1.49.0 Severity: Problem
Keywords: Cc:

Description

There are two places where the implementation of boost::function uses a user-provided allocator:

  1. boost/function/function_base.hpp, line 485
  2. boost/function/function_template.hpp, line 591

Both use cases do not protected against a possibly throwing copy constructor of a function-object because there is an unprotected two-step sequence of the following style:

wrapper_allocator_pointer_type copy = wrapper_allocator.allocate(1);
wrapper_allocator.construct(copy, functor_wrapper_type(f,a));

If the second step fails, no clean-up of the allocated memory takes place. The following test program emulates this situation and demonstrates the problem:

#include "boost/function.hpp"
#include <memory>
#include <iostream>

struct F {
  static bool dothrow;
  unsigned char prevent_short_object_opt[sizeof(boost::detail::function::function_buffer) + 1];
  F(){}
  F(const F&) {
    if (dothrow)
      throw 0;
  }
  void operator()() {}
};

bool F::dothrow = false;
int alloc_cnt = 0;

template<class T>
struct my_alloc : std::allocator<T>
{
  template<class Other>
  struct rebind
  {
    typedef my_alloc<T> other;
  };
  void construct(typename std::allocator<T>::pointer p, const T& val)
  {
    F::dothrow = true;
    ::new((void *)p) T(val);
  }
  void deallocate(typename std::allocator<T>::pointer p, 
	              typename std::allocator<T>::size_type n)
  {
    --alloc_cnt;
    std::cout << "deallocate: " << alloc_cnt << std::endl;
    return std::allocator<T>::deallocate(p, n);
  }

  typename std::allocator<T>::pointer allocate(typename std::allocator<T>::size_type n)
  {
    ++alloc_cnt;
    std::cout << "allocate: " << alloc_cnt << std::endl;
    return std::allocator<T>::allocate(n);
  }

};

int main() {
  F f;
  my_alloc<F> a;
  try {
    boost::function<void()> fu(f, a);
  } catch (int) {
    std::cout << "Caught expected - allocation count: " << alloc_cnt << std::endl; 
  }
}

The program outputs

allocate: 1
Caught expected - allocation count: 1

on all systems I tested (Visual Studio 11 beta and mingw with gcc 4.8) showing that the deallocation function is never called.

The two-step process of allocation+construct needs to be made exception-safe. In my own type-erased allocators I'm using a helper type

template<class Alloc>
struct allocated_ptr
{
  typedef typename Alloc::pointer pointer;

  explicit allocated_ptr(Alloc& alloc)
    : alloc(alloc), ptr(alloc.allocate(1))
  {}

  ~allocated_ptr()
  {
    if (ptr != pointer()) {
      alloc.deallocate(ptr, 1);
    }
  }

  pointer get() const { return ptr; }

  pointer release()
  {
    pointer result = ptr;
    ptr = pointer();
    return result;
  }

private:
  Alloc& alloc;
  pointer ptr;
};

to handle this, invoking the release function, after successful construction. Of-course other means are possible.

Change History (1)

comment:1 by Daniel Krügler <daniel.kruegler@…>, 10 years ago

I should mention that the original test contained a hack to simplify the allocator's emulation. To demonstrate that the test is not based on false usage of the allocators API here follows a corrected version, which does not behave differently in regard to the relevant point:

#include "boost/function.hpp"
#include <memory>
#include <iostream>

struct F {
  static bool dothrow;
  unsigned char prevent_short_object_opt[sizeof(boost::detail::function::function_buffer) + 1];
  F(){}
  F(const F&) {
    if (dothrow)
      throw 0;
  }
  void operator()() {}
};

bool F::dothrow = false;
int alloc_cnt = 0;

template<class T>
struct my_alloc : std::allocator<T>
{
  my_alloc() : std::allocator<T>() {}
  template<class U>
  my_alloc(const my_alloc<U>& other) : std::allocator<T>(other) {}
  template<class Other>
  struct rebind
  {
    typedef my_alloc<Other> other;
  };
  void construct(typename std::allocator<T>::pointer p, const T& val)
  {
    F::dothrow = true;
    ::new((void *)p) T(val);
  }
  void deallocate(typename std::allocator<T>::pointer p,
	              typename std::allocator<T>::size_type n)
  {
    --alloc_cnt;
    std::cout << "deallocate: " << alloc_cnt << std::endl;
    return std::allocator<T>::deallocate(p, n);
  }
  typename std::allocator<T>::pointer allocate(typename std::allocator<T>::size_type n)
  {
    ++alloc_cnt;
    std::cout << "allocate: " << alloc_cnt << std::endl;
    return std::allocator<T>::allocate(n);
  }
};

int main() {
  F f;
  my_alloc<F> a;
  try {
    boost::function<void()> fu(f, a);
  } catch (int) {
    std::cout << "Caught expected - allocation count: " << alloc_cnt << std::endl;
  }
}
Note: See TracTickets for help on using tickets.