Changes between Initial Version and Version 2 of Ticket #6305


Ignore:
Timestamp:
Mar 11, 2009, 2:59:19 PM (15 years ago)
Author:
Janne Grunau
Comment:

I don't think the patch for Recorderbase is correct. The QWaitconditions are used as interuptible sleep. There is no need for useful mutexes in this usage case. Another problem with the patch is that it doesn't change the usage in derived classes.

the member variables paused and request_pause could probably be guarded by mutexes which should be used in the wait conditions in recorderbase.

I'm ready to accept that our standard Qt4 port of QWaitCondition with adding a otherwise unused mutex might create deadlocks in some cases but recorderbase is a bad example.

Legend:

Unmodified
Added
Removed
Modified

  • Ticket #6305 – Description

    initial v2  
    22
    33Example:
    4 
     4{{{
    55Thread 12 (Thread 0x49396950 (LWP 11152)):
    66#0  0x00007fdd7ee1c2d9 in pthread_cond_wait@@GLIBC_2.3.2 () from /lib/libpthread.so.0
    … …  
    3838#32 0x00007fdd7ca4acbd in clone () from /lib/libc.so.6
    3939#33 0x0000000000000000 in ?? ()
     40}}}
    4041
    4142It looks there is multiple places where the QWaitCondition.wait/wakeall have been implemented incorrectly and therefore the code doesn't work as intended.
    4243
    4344Example:
    44 
     45{{{
    4546// Qt4 requires a QMutex as a parameter...
    4647// not sure if this is the best solution.  Mutex Must be locked before wait.
    … …  
    5455            mainpaused = true;
    5556            pauseWait.wakeAll();
     57}}}
    5658^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    5759No locks around this and no predicative set
    5860to make sure that the signal is detected in the receiver
    59 
     61{{{
    6062            if (IsPaused() && tvrec)
    6163                tvrec->RecorderPaused();
    6264 
    6365            unpauseWait.wait(&mutex, 100);
     66}}}
    6467^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    6568Uses a bogus mutex, doesn't have a proper locking around
    6669the wait-call and no predicative checking in order to detect
    6770if the wake-call has already been sent before we come to the wait()-call.
    68 
     71{{{
    6972            if (cleartimeonpause)
    7073                gettimeofday(&stm, &tzone);
    7174            continue;
     75}}}
    7276------------------------------------------------------------
    7377