HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Safe handling of variables in multi-threading application with shared resources

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
handlingmultiapplicationsharedwiththreadingvariablessaferesources

Problem

I've been developing this application as my first C++ project and am at a point where it is functional and would like to take a step back and critique/improve my code from a standpoint of manageability and better coding practices. Additionally, it has some stability issues which I need to address, so I thought instead of dumping a ton of code out there and asking for feedback we'd start at the beginning.

This application has multiple threads with shared resources, and sharing these resources across threads was particularly tricky for a novice programmer. So please see the below 'main.cpp' of my application that creates and waits for the destruction of all threads. Additionally these notes might be helpful:

  • The threads are meant to be asynchronous and operate at different rates. For example, the thread that updates the display operates a lower frame-rate than the thread that actually captures the image. I particularly wanted to avoid the scenario where the threads were locked together and the slowest thread set the pace for all others.



  • All threads are self exiting based on the 'exitsignal' variable. Essentially this code runs on raspberry pi in an automobile and when 12V signal is lost (car shut-off), a 3.3V input goes low and the GPIO thread writes 'exitsignal' true and all the other threads exit on their next iteration of their for-loop.



  • The GPIO thread is finally exited by the 'shutdownsignal' written at the end.



Thanks in advance, and all (constructive) input is appreciated.

```
int main()
{
std::cout exitsignal{ false };
std::atomic shutdownsignal{ false };
cv::Mat captureimage;
std::mutex capturemutex;
cv::Mat displayimage;
std::mutex displaymutex;
ProcessValues processvalues;

//Start threads

//Start image capture thread
std::thread t_imagecapture( CaptureImageThread,
&captureimage,
&capturemutex,
&exitsign

Solution

Don't use threads to scatter your timing schedule of independent actions

In most cases timing schedules can be solved with a simple main loop and time measurement like such:

bool exit = false; // In case you have concurrency here a `std::atomic`
                    // is fine of course
 auto least = std::chrono::high_resolution_clock::now();
 do {
      auto current = std::chrono::high_resolution_clock::now(); 
      auto timeDiff = current - least;
      if(timeDiff >= 20ms)  {
          // do the action every 20 milliseconds
      }
      if(timeDiff >= 1s)  {
          // do the action every second
      }
      // aso.
 } while(!exit);


Threads and synchronization mechanisms come with some cost by means of performance and memory overhead you may want to avoid.

Minimize thread interdependencies

It seems a number of processing steps in your example could be serialized trivially by calling these one after another (has finishied).

You're passing cv::Mat captureimage; as parameter all of the time, but the processing steps are separate threads that have to wait for each others processing step.

Such could be easier solved with a simple function calling sequence.

Reduce the number of threads to the parts that could be run independently

It seems your scenario can be reduced to 3 threads. One that takes the video capture input, one that does the image processing (depends if another CPU core is available), and the last one that feeds the video output.

You likely can synchronize all these using a (std::mutex protected) processing queue of std::queue frames.

In that mentioned scenario a std::condition_variable may come in handy to trigger the concurrently waiting std::threads execution, as soon a new captured frame is available, or it's readily processed for output.

Code Snippets

bool exit = false; // In case you have concurrency here a `std::atomic<bool>`
                    // is fine of course
 auto least = std::chrono::high_resolution_clock::now();
 do {
      auto current = std::chrono::high_resolution_clock::now(); 
      auto timeDiff = current - least;
      if(timeDiff >= 20ms)  {
          // do the action every 20 milliseconds
      }
      if(timeDiff >= 1s)  {
          // do the action every second
      }
      // aso.
 } while(!exit);

Context

StackExchange Code Review Q#151253, answer score: 2

Revisions (0)

No revisions yet.