2

I'm currently working on a project in C++ in which I have a list of structs stored in a vector that have a lot processing associated with them. In order to speed things up, I've chosen to split the program across multiple threads, and the lazy way I've chosen to do this is by adding a mutex to each struct in the standard library vector. This way I can have a number of threads iterate over the array and basically take ownership of the individual elements by calling mutex.try_lock(), and either completing the associated processing with that element, or moving onto the next open one.

Before we get started, note that the following actually does work.

#include <mutex>
#include <vector>

struct foo {
    int a;
    std::mutex b;
};

void working_demo () {
    // assign by initializer list
    foo f = {.a = 1};
}

int main () {
    working_demo();
}

So, I intend to populate my standard vector in a way very similar to above, and it doesn't work.

#include <mutex>
#include <vector>

struct foo {
    int a;
    std::mutex b;
};

void broken_demo () {
    std::vector<foo> bar;

    // assign by initializer list
    bar.push_back({.a = 1});
}

int main () {
    broken_demo();
}

Compiler Error:

clang++ -std=c++11 -Wall -Wextra -Wfatal-errors -pedantic -I./  -c -o demo.o demo.cpp
demo.cpp:13:20: warning: designated initializers are a C99 feature [-Wc99-extensions]
    bar.push_back({.a = 1});
                   ^~~~~~
In file included from demo.cpp:1:
In file included from /usr/sup/bin/../lib/gcc/x86_64-pc-linux-gnu/9.1.0/../../../../include/c++/9.1.0/mutex:38:
In file included from /usr/sup/bin/../lib/gcc/x86_64-pc-linux-gnu/9.1.0/../../../../include/c++/9.1.0/tuple:39:
In file included from /usr/sup/bin/../lib/gcc/x86_64-pc-linux-gnu/9.1.0/../../../../include/c++/9.1.0/array:39:
In file included from /usr/sup/bin/../lib/gcc/x86_64-pc-linux-gnu/9.1.0/../../../../include/c++/9.1.0/stdexcept:39:
In file included from /usr/sup/bin/../lib/gcc/x86_64-pc-linux-gnu/9.1.0/../../../../include/c++/9.1.0/string:41:
In file included from /usr/sup/bin/../lib/gcc/x86_64-pc-linux-gnu/9.1.0/../../../../include/c++/9.1.0/bits/allocator.h:46:
In file included from /usr/sup/bin/../lib/gcc/x86_64-pc-linux-gnu/9.1.0/../../../../include/c++/9.1.0/x86_64-pc-linux-gnu/bits/c++allocator.h:33:
/usr/sup/bin/../lib/gcc/x86_64-pc-linux-gnu/9.1.0/../../../../include/c++/9.1.0/ext/new_allocator.h:146:8: fatal error: call to implicitly-deleted copy constructor of 'foo'
                            _Up(std::forward<_Args>(__args)...)))
                            ^   ~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/sup/bin/../lib/gcc/x86_64-pc-linux-gnu/9.1.0/../../../../include/c++/9.1.0/bits/alloc_traits.h:483:24: note: in instantiation of exception specification for
      'construct<foo, foo>' requested here
        noexcept(noexcept(__a.construct(__p, std::forward<_Args>(__args)...)))
                              ^
/usr/sup/bin/../lib/gcc/x86_64-pc-linux-gnu/9.1.0/../../../../include/c++/9.1.0/bits/vector.tcc:115:21: note: in instantiation of exception specification for 'construct<foo, foo>'
      requested here
            _Alloc_traits::construct(this->_M_impl, this->_M_impl._M_finish,
                           ^
/usr/sup/bin/../lib/gcc/x86_64-pc-linux-gnu/9.1.0/../../../../include/c++/9.1.0/bits/stl_vector.h:1201:9: note: in instantiation of function template specialization
      'std::vector<foo, std::allocator<foo> >::emplace_back<foo>' requested here
      { emplace_back(std::move(__x)); }
        ^
demo.cpp:13:9: note: in instantiation of member function 'std::vector<foo, std::allocator<foo> >::push_back' requested here
    bar.push_back({.a = 1});
        ^
demo.cpp:6:16: note: copy constructor of 'foo' is implicitly deleted because field 'b' has a deleted copy constructor
    std::mutex b;
               ^
/usr/sup/bin/../lib/gcc/x86_64-pc-linux-gnu/9.1.0/../../../../include/c++/9.1.0/bits/std_mutex.h:94:5: note: 'mutex' has been explicitly marked deleted here
    mutex(const mutex&) = delete;
    ^

I'm fairly certain that this is saying the reason this won't work is that the structure is attempting to call the copy constructor for the mutex, which in fact does not have a copy constructor. I explicitly don't want to do be doing this.

My initial thought is that in order to make sure it doesn't even try to call the copy constructor for the mutex, I can create my own constructor for my class and basically explicitly leave out the copying for the mutex. This method would look like this - but it also doesn't work.

#include <mutex>
#include <vector>

struct foo {
    foo (int A): a(A) {;}

    int a;
    std::mutex b;
};

void broken_demo () {
    std::vector<foo> bar;

    // assign by initializer list
    bar.emplace_back(1);
}

int main () {
    broken_demo();
}

Compiler Error:

clang++ -std=c++11 -Wall -Wextra -Wfatal-errors -pedantic -I./  -c -o demo.o demo.cpp
In file included from demo.cpp:2:
In file included from /usr/sup/bin/../lib/gcc/x86_64-pc-linux-gnu/9.1.0/../../../../include/c++/9.1.0/vector:65:
/usr/sup/bin/../lib/gcc/x86_64-pc-linux-gnu/9.1.0/../../../../include/c++/9.1.0/bits/stl_construct.h:75:38: fatal error: call to implicitly-deleted copy constructor of 'foo'
    { ::new(static_cast<void*>(__p)) _T1(std::forward<_Args>(__args)...); }
                                     ^   ~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/sup/bin/../lib/gcc/x86_64-pc-linux-gnu/9.1.0/../../../../include/c++/9.1.0/bits/stl_uninitialized.h:83:8: note: in instantiation of function template specialization
      'std::_Construct<foo, foo>' requested here
                std::_Construct(std::__addressof(*__cur), *__first);
                     ^
/usr/sup/bin/../lib/gcc/x86_64-pc-linux-gnu/9.1.0/../../../../include/c++/9.1.0/bits/stl_uninitialized.h:134:2: note: in instantiation of function template specialization
      'std::__uninitialized_copy<false>::__uninit_copy<std::move_iterator<foo *>, foo *>' requested here
        __uninit_copy(__first, __last, __result);
        ^
/usr/sup/bin/../lib/gcc/x86_64-pc-linux-gnu/9.1.0/../../../../include/c++/9.1.0/bits/stl_uninitialized.h:289:19: note: in instantiation of function template specialization
      'std::uninitialized_copy<std::move_iterator<foo *>, foo *>' requested here
    { return std::uninitialized_copy(__first, __last, __result); }
                  ^
/usr/sup/bin/../lib/gcc/x86_64-pc-linux-gnu/9.1.0/../../../../include/c++/9.1.0/bits/stl_uninitialized.h:310:19: note: in instantiation of function template specialization
      'std::__uninitialized_copy_a<std::move_iterator<foo *>, foo *, foo>' requested here
      return std::__uninitialized_copy_a
                  ^
/usr/sup/bin/../lib/gcc/x86_64-pc-linux-gnu/9.1.0/../../../../include/c++/9.1.0/bits/vector.tcc:473:10: note: in instantiation of function template specialization
      'std::__uninitialized_move_if_noexcept_a<foo *, foo *, std::allocator<foo> >' requested here
                = std::__uninitialized_move_if_noexcept_a
                       ^
/usr/sup/bin/../lib/gcc/x86_64-pc-linux-gnu/9.1.0/../../../../include/c++/9.1.0/bits/vector.tcc:121:4: note: in instantiation of function template specialization 'std::vector<foo,
      std::allocator<foo> >::_M_realloc_insert<int>' requested here
          _M_realloc_insert(end(), std::forward<_Args>(__args)...);
          ^
demo.cpp:15:9: note: in instantiation of function template specialization 'std::vector<foo, std::allocator<foo> >::emplace_back<int>' requested here
    bar.emplace_back(1);
        ^
demo.cpp:8:16: note: copy constructor of 'foo' is implicitly deleted because field 'b' has a deleted copy constructor
    std::mutex b;
               ^
/usr/sup/bin/../lib/gcc/x86_64-pc-linux-gnu/9.1.0/../../../../include/c++/9.1.0/bits/std_mutex.h:94:5: note: 'mutex' has been explicitly marked deleted here
    mutex(const mutex&) = delete;
    ^
1 error generated.

Any thoughts on a solution to this? I'm hoping to keep the code relatively simple, but I'm uncertain how to make the standard vector behave, or at least use it properly.

Ashton Stephens
  • 31
  • 2
  • I don't think the `.a = 1` is legal in C++11 (Addition in C++20?). Been around in C for a while, so maybe allowed by extension. – user4581301 Nov 18 '19 at 05:38
  • 1
    You say the ``.a`` initialization works? Since when can you set individual fields in an initializer list? That's one thing highly useful in higher level languages that makes good sense in C++ and I've wanted implemented for a while. Have I missed something? And you're compiling with C++11 as well? – Pickle Rick Nov 18 '19 at 05:46
  • 1
    Slightly unrelated: [How should I deal with mutexes in movable types in C++?](https://stackoverflow.com/questions/29986208/how-should-i-deal-with-mutexes-in-movable-types-in-c) – user4581301 Nov 18 '19 at 05:53
  • @PickleRick Designated initialization looks like it will be added in C++20, but as far as I can tell only for [aggregate initialization](https://en.cppreference.com/w/cpp/language/aggregate_initialization). Doesn't make sense to me for any of the other [initialization types](https://en.cppreference.com/w/cpp/language/initialization). I suppose an `initializer_list` of aggregate initializers makes sense, but I haven't tried it yet. – user4581301 Nov 18 '19 at 06:15

2 Answers2

1

The problem is that in your code instances of foo are passed by value. So when you put something into the vector, a copy needs to be created. A simple way to avoid that would be to put pointers to foo into the vector. You can wrap the pointers into some reference-counting mechanism so that you don't have to keep track of freeing the objects.

Here is a short example using std::unique_ptr that will auto-delete all foo instances in the vector when the vector goes out of scope:

#include <mutex>
#include <vector>
#include <memory>
#include <iostream>
#include <functional>

struct foo {
   foo(int A) : a(A) {}
   int a;
   std::mutex b;
};

void fixed_demo () {
   std::vector<std::unique_ptr<foo>> bar;

   std::unique_ptr<foo> p(new foo(1));
   bar.push_back(std::move(p));
   bar.emplace_back(new foo(2));
   for (auto it = bar.begin(); it != bar.end(); ++it) {
      (*it)->b.lock();
      std::cout << (*it)->a << std::endl;
      (*it)->b.unlock();
   }
}

int main () {
   fixed_demo();
   return 0;
}
Daniel Junglas
  • 5,830
  • 1
  • 5
  • 22
1

After looking at one of the solutions, I think actually this solution fits best what I was going for. I wouldn't have been able to figure this out without the helpful advice above.

#include <mutex>
#include <vector>

struct foo {
    int a;
    std::mutex b;

    foo (int a) : a(a) {;} 
    foo (const foo &f) : a(f.a) {;}
};

void other_fixed_demo () 
{
    std::vector<foo> bar;
    bar.emplace_back(1);
}

int main () {
    other_fixed_demo();
}

I couldn't say why, but apparently two things are happening here. First, we are calling the constructor with the 1 input, and then somewhere within emplace_back() we're calling the copy constructor. Both need to explicitly not include the mutex in each way of creating the struct, and the above solution avoids that.

Ashton Stephens
  • 31
  • 2