Thread: std::thread help!!!

  1. #1
    Registered User MutantJohn's Avatar
    Join Date
    Feb 2013
    Posts
    2,665

    std::thread help!!!

    Hello All,

    I'm attempting to write some multithreaded code and it ain't goin' swell.

    Okay, this should be all the relevant code you need :
    Code:
    mutex ins, surf, out;
    
    
    void counter(int & inside, int & surface, int & outside, vector<particle> parts, tetra *root) {
    
    
          int x = 0;
    
    
          for (auto it = parts.begin(); it < parts.end(); ++it) {
    
    
             x = inside_tetra(root, &(*it));
    
    
             if (x == 0) {
    
    
                ins.lock();
                ++inside;
                ins.unlock();
             }
    
    
             if (x == -1) {
    
    
                out.lock();
                ++outside;
                out.unlock();
             }
    
    
             if (x > 0) {
    
    
                surf.lock();
                ++surface;
                surf.unlock();
             }
          }
    
    
          return;
       }
    
    /* ... now we're in the main-loop of the code.. */
    
       thread *threads = new thread[num_threads];
       vector<particle> *particle_sets = new vector<particle>[num_threads];
    
       for (unsigned i=0; i < num_threads; ++i) {
    
    
          threads[i] = thread(counter, inside, surface, outside, particle_sets[i], root_tetra);
       }
    Now, a first reaction would be, MutantJohn, you sexy beast, why are you allocating memory for threads and particle_sets. The answer is because doing otherwise does not comply with the ISO standard and I'm all about that.

    Basically, my code won't compile and I have no idea why. Oh, and root_tetra is a valid allocation in memory. The output I'm getting is really weird, it's basically :
    Code:
    g++ -g -O3 -Wall -Wextra -std=c++11 -lpthread -pedantic `pkg-config --cflags --libs gsl` -c main.cpp
    In file included from /usr/include/c++/4.8.2/thread:39:0,
                     from structures.h:7,
                     from main.cpp:1:
    /usr/include/c++/4.8.2/functional: In instantiation of ‘struct std::_Bind_simple<void (*(int, int, int, std::vector<particle>, tetra*))(int&, int&, int&, std::vector<particle>, tetra*)>’:
    /usr/include/c++/4.8.2/thread:137:47:   required from ‘std::thread::thread(_Callable&&, _Args&& ...) [with _Callable = void (&)(int&, int&, int&, std::vector<particle>, tetra*); _Args = {int&, int&, int&, std::vector<particle, std::allocator<particle> >&, tetra*&}]’
    main.cpp:120:90:   required from here
    /usr/include/c++/4.8.2/functional:1697:61: error: no type named ‘type’ in ‘class std::result_of<void (*(int, int, int, std::vector<particle>, tetra*))(int&, int&, int&, std::vector<particle>, tetra*)>’
           typedef typename result_of<_Callable(_Args...)>::type result_type;
                                                                 ^
    /usr/include/c++/4.8.2/functional:1727:9: error: no type named ‘type’ in ‘class std::result_of<void (*(int, int, int, std::vector<particle>, tetra*))(int&, int&, int&, std::vector<particle>, tetra*)>’
             _M_invoke(_Index_tuple<_Indices...>)
             ^
    Makefile:18: recipe for target 'main.o' failed
    make: *** [main.o] Error 1

  2. #2
    Registered User MutantJohn's Avatar
    Join Date
    Feb 2013
    Posts
    2,665
    Okay, just a normal function call will work.

    This :
    Code:
    counter(inside, surface, outside, particle_sets[0], root_tetra);
    will actually compile just fine when I'm not trying to launch threads.

  3. #3
    Registered User MutantJohn's Avatar
    Join Date
    Feb 2013
    Posts
    2,665
    Okay, I'm a moron. Or I learned something about the STL today. You cannot pass references as arguments when using std::threads. Instead, you should use pointer syntax because that will compile. Oh. My. God.

    Sorry, everybody. Nothin' to see here T_T

  4. #4
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    Quote Originally Posted by MutantJohn View Post
    Okay, I'm a moron. Or I learned something about the STL today. You cannot pass references as arguments when using std::threads. Instead, you should use pointer syntax because that will compile. Oh. My. God.
    Well, actually you can. It just won't do what you expect:

    Code:
    #include <thread>
    
    void foo2(int&) {}
    
    void foo(int& n) 
    {
    	std::cout << "foo: n is " << n << ".\n";
    	n = 10;
    	std::cout << "foo: n is " << n << ".\n";
    
    	std::thread thread(&foo2, n);
    	thread.join();
    }
    
    int main()
    {
    	int n = 0;
    	std::cout << "main: n is " << n << ".\n";
    	std::thread thread(&foo, n);
    	thread.join();
    	std::cout << "main: n is " << n << ".\n";
    }
    Output:
    main: n is 0.
    foo: n is 0.
    foo: n is 10.
    main: n is 0.

    To fix, change
    std::thread thread(&foo, n);
    to
    std::thread thread(&foo, std::ref(n));

    However, a couple of points:
    You can simplify
    for (auto it = parts.begin(); it < parts.end(); ++it)
    to
    for (const auto & val : parts)
    (assuming you don't need the iterator)

    You should not lock manually, so don't write

    ins.lock();
    ++inside;
    ins.unlock();

    Instead, use a lock, ie unique_lock:

    std::mutex m;
    std::unique_lock<std::mutex> l(m);

    This takes the RAII principle into account and unlocks upon destruction of the lock.
    But if you are just going to increment a variable, you may want to use atomics instead.

    std::atomic<int> MyAtomicInt(0);
    ++MyAtomicInt;

    Why on earth are you manually allocating memory with new? You should be using a vector, eg:

    std::vector<std::thread> v;
    v.emplace_back(&foo, std::ref(n));
    v[0].join();

    You can use a 2D-vector too, or just a plain struct to group thread and thread data, so

    std::vector<std::vector<particle>> v(N, std::vector<particle>(N));

    Or a struct

    Code:
    struct Thread
    {
        // TODO: Create constructor
        std::thread Thread;
        particle Particle;
    }
    
    std::vector<Thread> MyThreads;
    MyThreads.emplace_back(...);
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Replies: 3
    Last Post: 11-20-2011, 12:01 AM
  2. Replies: 2
    Last Post: 07-01-2007, 07:11 AM
  3. Message box as thread to stop main thread
    By Yasir_Malik in forum Windows Programming
    Replies: 8
    Last Post: 04-11-2006, 11:07 AM
  4. pointer to main thread from worker thread?
    By draegon in forum C++ Programming
    Replies: 2
    Last Post: 10-27-2005, 06:35 AM
  5. Replies: 2
    Last Post: 04-12-2004, 01:37 AM