Boost promise & future does not use supplied allocator for value storage

[Niall Douglas]

Presently Boost.Thread's promise and future does not use the constructor supplied allocator instance to allocate the shared state value, and instead uses operator new. This breaks with the C++11 standard behaviour.

Niall

Copy and paste from email dialogue with Vicente:

Le 20/11/13 18:07, Niall Douglas a écrit :

Vicente,

Just checking something: VS2013 is giving me a warning about unaligned allocation when I try to do this:

1. I have BOOST_THREAD_PROVIDES_FUTURE_CTOR_ALLOCATORS defined.

2. I initialise a promise<Int256> like this:

boost::promise<Int256_type>(boost::allocator_arg_t(), aligned_allocator<Int256, 32>())

3. When I try to use set_value() on the promise, I then see the warning

due to this function around line 551 in struct future_traits:

static void init(storage_type& storage,source_reference_type t) {

storage.reset(new T(t));

}

which implies that the allocator configured for the type Int256 is not used because you are using operator new instead of the allocator?

Maybe this is me misunderstanding how futures and promises use an externally supplied allocator. Your future/promise correctly uses the allocator to initialise shared_state, but internally to shared_state it then creates a scoped_ptr to hold the actual value storage and that is where operator new is being invoked to store the value instead of the allocator.

If this was not intentional, can I suggest that instead of keeping a scoped_ptr to the value, you could instead keep the value itself stored in shared_state which then would have the correct alignment? You could use placement new and placement delete to construct/destruct the instance without affecting storage with a separate flag elsewhere indicating validity. Alternatively, I'm sure there must be some way of preallocating shared_state storage while the type of the custom allocator is still available.

Thanks, Niall

Hi Niall,

you are right. I missed this allocation.

I'm too busy currently. It would be greate if you have the time to provide a patch.

Best, Vicente

P.S. Please, create a ticket so that I don't forget this issue.

[viboes]

Moved to problem as this doesn't prevent the use of the library.

[viboes]

After a first analysis, I think that we need to use unique_ptr<T> instead of scoped_ptr<T>. The problem is that we need a type erased deleter so that we can instantiate with default_delete<T> and with a specific allocator_delete<T,A>.

unique_ptr<T, erased_delete<T>>

I will start by refactoring the code, so that it is possible to set the value using an erased unique_ptr<> instead of the current source_reference_type and rvalue_source_type.

[viboes]

After some trials, I have fall on issues mixing the old Boost.Thread move emulation and boost::movelib_unique_ptr<T,D>, which is of course not aware of this old emulation.

I suspect that this issue could only be fixed on a version that doesn't supports the old move emulation.

I'm wondering if it will be a good idea to branch a specific future.hpp implementation that doesn't supports the old move semantics and is aligned with version 4.

[Niall Douglas]

FYI Vicente I have finished concurrent_unordered_map. My next task is non-allocating future-promise based on expected<T, E>, and I have been in discussions with Louis Dionne about the monadic part of the design as I do not fully understand it. Once those are complete I shall begin work.

Niall

[viboes]

An alternative could be to use optional<T> (waiting for expected<T>).

With optional<T> there will be no internal allocation, and so the issue will be a non-issue.

[viboes]

Unfortunately, optional<T> doesn't supports emulated move semantics with Boost.Move. This means that optional<T> could be used only on C++11 compilers supporting move semantics.

Niall, would this be enough for you?

[viboes]

​https://github.com/boostorg/thread/commit/f73007910af7a5ef68bc5d722d262b54dd0cf3ca

[Niall Douglas]

To be clear, I personally no longer need allocating shared_ptr now the non-allocating future-promise is shortly to become real. My original need for allocating shared_ptr was to avoid unnecessary memory allocation, so I could recycle the buffers. That won't be necessary soon as there will be no buffers.

Also, if I am successful, I should hope the C++ 17 standard will change promise-future to match ours. And then allocation will become deprecated.

[viboes]

Ok. I consider then that this bug is closed.