r15020 causes backend segfault on startup

[mythtv@…]

mythbackend segfaults on startup post r15020 with the error:

QThread object destroyed while thread is still running.

The attached patch resolves the issue for me.

I believe that the specific issue was that the SSDP::RequestTerminate?() function contained a QThread::wait(500) call (which is wait 0.5 seconds for the thread to exit) but the SSDP::run() function (the thread loop) contains a blocking "select" call with a timeout of 1 second, meaning the probability of the thread still executing (waiting for a second) is very high when the object is deleted.

So, the specific bug fix is actually to remove the timeout from the QThread::wait() calls. The other changes in the patch are other things I noticed whilst investigating the code. Specifically:

• Mark m_bTermRequested members as "volatile"; Which instructs the compiler that their values may change outside of the normal flow of control (because they are accessed by two separate threads)

• Eliminated IsTermRequested??() member functions; replacing these with direct references to the m_bTermRequested member variables. There is no point in protecting read accesses to these members by mutexs because doing so will make no difference to the results, it merely adds overhead.

• Eliminated mutex locking in Task::Task(), this wasn't doing anything useful.

• Eliminated RequestTerminate??() member functions; It didn't strike me as sensible to enforce callers to have to call extra functions on an object prior to destroying it (and introducing the possibilities of callers failing to call these important functions) this functionality should reside in the class' destructors.

[mythtv@…]

Updated patch includes reverting 15020

[mythtv@…]

After giving it some more thought, I concluded that deleting TaskQueue? object before the SSDP object is not thread safe, so I've updated the patch to revert back to the original ordering (prior to commit 15020).

[Nigel]

Peter, thanks for your analysis and patch. I have applied it locally, and am experimenting. Sadly, there are still (other?) bugs:

Thread 1:
0   libSystem.B.dylib        	0x90047dd7 semaphore_timedwait_signal_trap + 7
1   qt-mt                    	0xb22a98a4 QWaitCondition::wait(QMutex*, unsigned long) + 242
2   mythupnp                 	0x047b9245 CEvent::WaitForEvent(unsigned long) + 85 (threadpool.cpp:96)
3   mythupnp                 	0x047bac8b WorkerThread::run() + 181 (threadpool.cpp:202)
4   qt-mt                    	0xb203110f QThreadInstance::start(void*) + 111
5   libSystem.B.dylib        	0x90024227 _pthread_body + 84

Thread 2 Crashed:
0   qt-mt                    	0xb22a86ae QMutex::lock() + 14
1   mythtv                   	0x017de6c4 RefCounted::AddRef() + 20 (refcounted.h:50)
2   mythupnp                 	0x047e0d85 SSDPCacheEntries::RemoveStale(timeval const&) + 99 (ssdpcache.cpp:160)
3   mythupnp                 	0x047e0f61 SSDPCache::RemoveStale() + 135 (ssdpcache.cpp:395)
4   mythupnp                 	0x047ee52f SSDPCacheTask::Execute(TaskQueue*) + 45 (upnptaskcache.h:53)
5   mythupnp                 	0x047ae09d TaskQueue::run() + 109 (taskqueue.cpp:104)
6   qt-mt                    	0xb203110f QThreadInstance::start(void*) + 111
7   libSystem.B.dylib        	0x90024227 _pthread_body + 84

[Nigel]

(In [15184]) Thread-safety fixes for UPnP destruction, thanks to Peter Stokes. Closes #4301