0

I have a Visual Studio project that worked fine until I tried to implement multithreading. The project acquires images from a GigE camera, and after acquiring 10 images, a video is made from the acquired images.

The program flow was such that the program didn't acquire images when it was making the video. I wanted to change this, so I created another thread that makes the videos from the images. What I wanted is that the program will acquire images continuously, after 10 images are acquired, another thread runs in parallel that will make the video. This will continue until I stop the program, 10 images are acquired, video from these 10 images is made in parallel while the next 10 images are acquired and so on.

I haven't created threads before so I followed the tutorial on this website. Similar to the website, I created a thread for the function that saves the video. The function that creates the video takes the 10 images as a vector argument. I execute join on this thread just before the line where my main function terminates.

For clarity, here's pseudo-code for what I've implemented:

#include ...
#include <thread>

using namespace std;

thread threads[1];
vector<Image> images;

void thread_method(vector<Image> & images){
    // Save vector of images to video
    // Clear the vector of images
}

int main(int argc, char* argv[]){
    // Some stuff
    while(TRUE)
    {
        for (int i = 0; i < 10; i++)
        {    
            //Acquire Image
            //Put image pointer in images vector named images
        }
        threads[0] = thread(thread_method, images)
    }

    // stuff

    threads[0].join();

    cout << endl << "Done! Press Enter to exit..." << endl;
    getchar();

    return 0;
}

When I run the project now, a message pops up saying that the Project.exe has triggered a breakpoint. The project breaks in report_runtime_error.cpp in static bool __cdecl issue_debug_notification(wchar_t const* const message) throw().

I'm printing some cout messages on the console to help me understand what's going on. What happens is that the program acquires 10 images, then the thread for saving the video starts running. As there are 10 images, 10 images have to be appended to the video. The message that says Project.exe has triggered a breakpoint pops up after the second time 10 images are acquired, at this point the parallel thread has only appended 6 images from the first acquired set of images to the video.

The output contains multiple lines of thread XXXX has exited with code 0, after that the output says

Debug Error!

Program: ...Path\Program.exe

abort() has been called

(Press Retry to debug the application)

Program.exe has triggered a breakpoint.

db7638
  • 410
  • 6
  • 17
  • @Ron I've replaced `runStatus` with `TRUE`, `result` with 0, and added a line for the definition of `images`. `Image` is a class in an API (Camera API) that's unfortunately not publicly available. – db7638 Jul 27 '17 at 21:07

2 Answers2

1

You are overwriting your one thread in the while loop. If it's still running, the program is aborted. You have to join or detach each thread value.

You could instead do

#include // ...
#include <thread>

// pass by value, as it's potentially outliving the loop
void thread_method(std::vector<Image> images){
    // Save vector of images to video
}

int main(int argc, char* argv[]){
    // Some stuff
    while(TRUE)
    {
        std::vector<Image> images; // new vector each time round

        for (int i = 0; i < 10; i++)
        {    
            //Acquire Image
            //Put image pointer in images vector named images
        }
        // std::thread::thread will forward this move
        std::thread(thread_method, std::move(images)).detach(); // not join
    }

    // stuff

    // this is somewhat of a lie now, we have to wait for the threads too
    std::cout << std::endl << "Done! Press Enter to exit..." << std::endl;
    std::getchar();

    return 0;
}
Caleth
  • 52,200
  • 2
  • 44
  • 75
  • You may want to change to a `std::vector>` and move it into the thread, to avoid copying the images – Caleth Jul 27 '17 at 21:27
  • This worked! Although now I'm encountering another unrelated error, time for more debugging! – db7638 Jul 27 '17 at 21:38
  • Caleth: take a look at `std::ref` as a possible solution to the copying problem. But... zoinks! You want the copy. @db7638 do a serious rethink on what it looks like you are trying to do here. With a reference to the vector every thread is using the same vector. Early threads will be using the vector *while* elements are being added for later threads. vector resizes will be murderous as the data one thread is examining will disappear mid-use. Plus it is Majorly bad juju to have many threads simultaneously manipulating the same data. – user4581301 Jul 27 '17 at 21:50
  • Actually, scratch the unique_ptr, I just realised all you need is to std::move into the thread constructor – Caleth Jul 28 '17 at 08:09
1

I can't explain all this in a comment. I'm dropping this here because it looks like OP is heading in some bad directions and I'd like to head him off before the cliff. Caleth has caught the big bang and provided a solution for avoiding it, but that bang is only a symptom of OP's and the solution with detach is somewhat questionable.

using namespace std;

Why is "using namespace std" considered bad practice?

thread threads[1];

An array 1 is pretty much pointless. If we don't know how many threads there will be, use a vector. Plus there is no good reason for this to be a global variable.

vector<Image> images;

Again, no good reason for this to be global and many many reasons for it NOT to be.

void thread_method(vector<Image> & images){

Pass by reference can save you some copying, but A) you can't copy a reference and threads copy the parameters. OK, so use a pointer or std::ref. You can copy those. But you generally don't want to. Problem 1: Multiple threads using the same data? Better be read only or protected from concurrent modification. This includes the thread generating the vector. 2. Is the reference still valid?

    // Save vector of images to video
    // Clear the vector of images
}

int main(int argc, char* argv[]){
    // Some stuff
    while(TRUE)
    {
        for (int i = 0; i < 10; i++)
        {    
            //Acquire Image
            //Put image pointer in images vector named images
        }
        threads[0] = thread(thread_method, images)

Bad for reasons Caleth covered. Plus images keeps growing. The first thread, even if copied, has ten elements. The second has the first ten plus another ten. This is weird, and probably not what OP wants. References or pointers to this vector are fatal. The vector would be resized while other threads were using it, invalidating the old datastore and making it impossible to safely iterate.

    }

    // stuff

    threads[0].join();

Wrong for reasons covered by Caleth

    cout << endl << "Done! Press Enter to exit..." << endl;
    getchar();

    return 0;
}

The solution to joining on the threads is the same as just about every Stack Overflow question that doesn't resolve to "Use a std::string": Use a std::vector.

#include <iostream>
#include <vector>
#include <thread>


void thread_method(std::vector<int> images){
    std::cout << images[0] << '\n'; // output something so we can see work being done.
    // we may or may not see all of the numbers in order depending on how 
    // the threads are scheduled.    
}

int main() // not using arguments, leave them out.
{
    int count = 0; // just to have something to show

    // no need for threads to be global.
    std::vector<std::thread> threads; // using vector so we can store multiple threads

    // Some stuff
    while(count < 10) // better-defined terminating condition for testing purposes
    {
        // every thread gets its own vector. No chance of collisions or duplicated 
        // effort
        std::vector<int> images; // don't have Image, so stubbing it out with int
        for (int i = 0; i < 10; i++)
        {
            images.push_back(count);
        }
        // create and store thread.
        threads.emplace_back(thread_method,std::move(images));
        count ++;
    }

    // stuff

    for (std::thread &temp: threads) // wait for all threads to exit
    {
        temp.join();
    }

    // std::endl is expensive. It's a linefeed and s stream flush, so save it for 
    // when you really need to get a message out immediately
    std::cout << "\nDone! Press Enter to exit..." << std::endl; 

    char temp;
    std::cin >>temp; // sticking with standard librarly all the way through

    return 0;
}

To better explain

threads.emplace_back(thread_method,std::move(images));

this created a thread inside threads (emplace_back) that will call thread_method with a copy of images. Odds are good that the compiler would have recognized that this was the end of the road for this particular instance of images and eliminated the copying, but if not, std::move should give it the hint.

user4581301
  • 33,082
  • 7
  • 33
  • 54
  • This answer contains a lot of useful information on top of what I asked for. Should I still be clearing the images vector in `thread_method` after making the video from that images vector? I assume it doesn't matter much because I'm passing a copy now? – db7638 Jul 28 '17 at 17:31
  • No need to clear it. It will be destroyed on function exit, freeing up all the resources. The source `vector` in the `while` loop will also be destroyed and recreated every loop, so no need for clearing there. – user4581301 Jul 28 '17 at 17:48