0

I am programming a class which should use multithreading features. The goal is that I don't have any blocking methods from the outside although I use libraries in the class that have blocking functions. I want to run them in their own threads. Unfortunately I get a memory fault (core dumped) error.

What would be the best practice in c++11 to implement something like this and why do I get the error,how can I specify the memory for the function I want to call in the thread best in advance?

My Class

.. 
class foo {

  void startUp();

  foo();
  ~foo();
  
  std::thread foo_worker;
  int retValue;

};

void foo::startUp() {
  int retValue = 0;

  std::thread t([this] () {
  retValue = blocking_lib_func();
});

foo_worker = std::move(t);

}

foo::~foo() {
  ....
  foo_worker.join();
}

My Main

int main() 

  foo test();
  test.statUp()
}
  • 2
    Your question is too vague to answer, and you cannot claim to be non-blocking in your question and then `join` on a thread in your code. Suggest you look into `std::async` or more serious IPC libraries. – Botje Dec 14 '21 at 10:20
  • 3
    Why are you using so much dynamic memory allocation in these few lines? Also what do you think you are doing by making the member `volatile`? These are all indicators of bad programming habits that could easily lead to problems down the line – UnholySheep Dec 14 '21 at 10:24
  • Ok, so yes the .join() should be moved in to the destructor. I making the worker thread volatile cause this thread is controling a gui framework, to make ist "faster" – zombieanfuehrer Dec 14 '21 at 10:33
  • 3
    You are not making the thread `volatile` you are making the pointer to the thread `volatile` - and that has absolutely nothing to do with performance – UnholySheep Dec 14 '21 at 10:36
  • 4
    `volatile ` - _"...This makes volatile objects suitable for communication with a signal handler, __but not with another thread of execution__..."_ see https://en.cppreference.com/w/cpp/language/cv – Richard Critten Dec 14 '21 at 10:45

1 Answers1

1

The lambda associated with your thread is capturing a reference to local stack variable. When startUp returns, the thread will continue on, but the address of retValue is now off limits, including to the thread. The thread will create undefined behavior by trying to assign something to that reference to retValue. That is very likely the source of the crash that you describe. Or worse, stack corruption on the main thread corrupting your program in other ways.

The solution is simple. First, make retValue a member variable of your class. And while we are at it, no reason for foo_worker to be a pointer.

class foo {
public:
  void startUp();

  foo();
  ~foo();

private:      
  std::thread foo_worker;
  int retValue;

};

Then your startUp code can be this. We can use std::move to move the thread from the local t thread variable to the member variable of the class instance.

void foo::startUp() {

  std::thread t([this] () {
      retValue = blocking_lib_func(); // assign to the member variable of foo
  });
  foo_worker = std::move(t);
}

Then your destructor can invoke join as follows:

foo::~foo() {
  ....
  foo_worker.join();
}

And as other in the comments have pointed out, volatile isn't useful. It's mostly deprecated as a keyword when proper thread and locking semantics are used.

selbie
  • 100,020
  • 15
  • 103
  • 173
  • Ok thanks for the good explication :) I removed all the volatiles which was in here, also made the variable retValue a class member and did the thread to a normal declaration. But still the same error .. :( – zombieanfuehrer Dec 14 '21 at 12:05
  • I have the problem that the error(Memory fault (core dumped)) comes as soon as the blocking function is executed. . – zombieanfuehrer Dec 14 '21 at 13:03
  • 1
    I think problem must lie with what blocking_lib_func() does. The gui actions in there might not work if called from non-main thread. – WilliamClements Dec 14 '21 at 15:20
  • 1
    @zombieanfuehrer - I stand by this answer with regards to using std::thread with variables, but @WilliamClements is also correct. Most GUI framework functions aren't meant to be called but from the main application thread. Perhaps if you shed more light on what `blocking_lib_func();` is doing, we can assist more. – selbie Dec 14 '21 at 18:49
  • Hi, blocking_lib_func() will call a Qt mainwindow loop and handle signals and slots. This lib is a interface with the gui software, there is a also a deamon which should interact with this blocking_lib_func(). When I have a system I want the main thread to start different threads, among others the GUI which is only a part of my system. And not the main thread hanging on the GUI :( Or is there an design pattern which can help me to solve this problem? – zombieanfuehrer Dec 15 '21 at 08:47
  • There have been many questions here about threads with qt. Here is one: https://stackoverflow.com/questions/14545961/modify-qt-gui-from-background-worker-thread – WilliamClements Dec 15 '21 at 14:08
  • I suppose you *can* run your Qt GUI on a dedicated thread instead of the main thread, but that's not the standard pattern. – selbie Dec 15 '21 at 15:29
  • OK the failure is in the library function. With an older Version of the lib your solution will work! Thanks for the tipp top work with std::move and threads which part of my class! This will definitely help me more often – zombieanfuehrer Dec 15 '21 at 16:13