[mythtv-commits] Ticket #8872: Unable to access MythWeb when streaming UPNP

[MythTV]

#8872: Unable to access MythWeb when streaming UPNP
------------------------------------+---------------------------------------
 Reporter:  ccarter0@…              |           Owner:  beirdo    
     Type:  defect                  |          Status:  closed    
 Priority:  trivial                 |       Milestone:  0.24      
Component:  MythTV - UPnP           |         Version:  0.23-fixes
 Severity:  low                     |      Resolution:  fixed     
 Keywords:                          |   Ticket locked:  0         
------------------------------------+---------------------------------------

Comment (by Tomi Orava <tomi.orava@…>):

 I just happened to check the latest commit you did to threadpool.cpp and
 noticed that just above the line you added:

 VERBOSE(VB_IMPORTANT, QString("ThreadPool:%1: thread pool exhausted (max
 %2 threads)") .arg(m_sName) .arg(m_nMaxThreadCount));

 There is a code block originally made by Janne Grunau, that I've seen all
 over the mythtv code and it's completely wrong! ie:

 Around line 392 in trunk there is this code:



 {{{
 QMutex mutex;
 mutex.lock();

 if (m_threadAvail.wait( &mutex, 5000 ) == false )
 {
     VERBOSE(VB_IMPORTANT, QString("ThreadPool:%1: thread pool exhausted
 (max %2 threads)") .arg(m_sName) .arg(m_nMaxThreadCount));
     return( NULL );     // timeout exceeded.
 }

 }}}

 The thing is you just can't invent a temporary mutex for the condition
 wait and condition signal calls.
 Because the mutex is completely bogus and the threads sending the
 condition signals are not locking the very same mutex and also because
 there is no mutual signal variable between the signalling threads and the
 waiter it can happen very easily that the condition signal has already
 been sent _before_ the waiter thread calls the condition wait ---> the
 condition wait will eventually exit with timeout exceeded ie NULL value.

 All of this is very easy to fix, make all the signalling threads lock the
 mutex, set a mutual signalling variable (bool) to true, unlock the mutex
 and call condition.signal. The waiter then has to lock the common mutex,
 check if the mutual variable has been set to true --> a signal has been
 delivered beforehand, if true, set the variable to false, unlock the mutex
 and return with success etc. Otherwise the waiter just has to call the
 wait and if it receives a signal it just has to disable the signalling
 variable, unlock the mutex and return accordingly.

 I don't know it this actually helps in your ticket, but for sure this
 block contains a design bug.

-- 
Ticket URL: <http://svn.mythtv.org/trac/ticket/8872#comment:21>
MythTV <http://www.mythtv.org/>
MythTV Media Center