Util::Timer::Loop crash since replacement of boost::scoped_lock to std::lock_guard about common HOT 18 CLOSED

Yes, before delete.

from common.

Maybe a _thread->join() is required prior to deleting it. Can you try that? I think that std::thread is more sensitive to that than boost::thread.

from common.

OK thanks but where should I place _thread->join() exactly ?
In Timer::Loop at line: 355, so before the delete _thread ?

from common.

Unfortunately it crashes with error (in Terminal):
libc++abi: terminating due to uncaught exception of type std::__1::system_error: thread::join failed: Resource deadlock avoided

from common.

Then we have to test the code and fix it. If you need it right now, you can revert the commits which replaced boost with std.

from common.

Regarding your question regarding testing: we only test on Linux. All other platforms are experimental for which we do not provide builds.

from common.

If I comment out the part then no crashes anymore, but I'm aware that we get a memory leaks:

#if ( _thread ) {
#  _thread->join();
#   delete _thread;
#  _thread = nullptr;
#	}

So I moved it to Timer::deactivate() and when I close the window by clicking on the red button of a GUI-app like scrttv then the _thread gets properly deleted.

#if !defined(SC_HAS_TIMER_CREATE)
bool Timer::deactivate(bool remove) {
	assert(_isActive == true);

	_isActive = false;

	if ( remove ) {
		const std::lock_guard<std::mutex> lk(_mutex);

		for ( TimerList::iterator it = _timers.begin(); it != _timers.end(); ++it ) {
			if ( *it == this ) {
				_timers.erase(it);
				break;
			}
		}
		
	}
	
	if ( _thread ) {
		printf("DEBUG: Calling _thread->join\n");
		_thread->join();	
		delete _thread;
		_thread = nullptr;
	}

	return true;
}

Could this be the solution ?

from common.

It only works if there is only one timer. Two or more timers won't work. Under BSD exists the callout API for timers. But I cannot test it due to the lack of access to such systems.

from common.

The issue is that you are not allowed to delete a thread within a running thread. I could reproduce it and trying to fix it.

from common.

Actually your fix was almost correct, I have corrected it slightly. Please give it a try.

from common.

OK will test it...also shouldn't the thread be checked if it is joinable and then call detach() ? This works for me in the Timer::Loop

if(_thread->joinable() ){
		_thread->detach();	
		//delete _thread;
		//_thread = nullptr;
	}

from common.

joinable() could be checked in the code I have changed, prior to join().

from common.

Your code works now! thanks! No more crashes! But yes I changed:

if ( _timers.empty() && _thread {
...
}

to:

if ( _timers.empty() && _thread->joinable() ) {

}

Is this correct ?

Also would it be better to use std::this_thread::sleep_for(std::chrono::seconds(1)); in Timer::Loop instead of Core::sleep()?
Does Core::sleep() use usleep() ?

from common.

_thread might be nullptr, the check is required. Add joinable() as condition for join(). Feel free to play with different sleep methods.

from common.

Ok this should be better:

if ( _timers.empty() && _thread ) {
		if(_thread->joinable() ){
			_thread->join();
			delete _thread;
			_thread = nullptr;
		}	
	}

from common.

Almost ... it has to be deleted, whether joined or not.

from common.

Strangely enough it never gets into this part of code in Timer::deactivate when closing a GUI-app or making an seiscomp update-config... Changed again:

if ( _timers.empty() && _thread ) {
		printf("DEBUG: _timers.empty() && _thread  ?");
		
		if(_thread->joinable() ){ 
			_thread->join();
		}
		
		printf("DEBUG: Delete thread");	
		delete _thread;
		_thread = nullptr;
			
	}

from common.

Thanks for your fixes with commits:
c94241f
bd650c5

No more Timer related crashes for macOS.
This issue can be closed.

from common.