Что небезопасно в этой чрезвычайно простой реализации потока мониторинга? - PullRequest
Новая
       13

Что небезопасно в этой чрезвычайно простой реализации потока мониторинга?

2 голосов
/ 27 января 2010

Я новичок в синхронизации потоков. Я читал много реализаций условных переменных, таких как boost :: threads и pthread для win32. Я только что реализовал этот довольно простой монитор с помощью wait / notify / noifyall, и я думаю, что у него много скрытых проблем, которые я хотел бы обнаружить у более опытных людей. Любое предложение? 

class ConditionVar
{

public :
    ConditionVar () : semaphore ( INVALID_HANDLE_VALUE ) , total_waiters (0)
    {
        semaphore = ::CreateSemaphoreA ( NULL , 0 /* initial count */ , LONG_MAX /* max count */ , NULL );
    }

    ~ConditionVar ()
    {
        ::CloseHandle ( semaphore ) ;       
    }


public :
    template <class P>
    void Wait ( P pred )
    {
        while ( !pred() ) Wait();
    }

public :

    void Wait ( void )
    {
        INTERLOCKED_WRITE_RELEASE(&total_waiters,total_waiters + 1 );
        ::WaitForSingleObject ( semaphore , INFINITE );
    }

    //! it will notify one waiter
    void Notify ( void )
    {
        if ( INTERLOCKED_READ_ACQUIRE(&total_waiters) )
        {
            Wake (1);
        }
    }

    void NotifyAll (void )
    {
        if ( INTERLOCKED_READ_ACQUIRE(&total_waiters) )
        {
            std::cout << "notifying " << total_waiters ;
            Wake ( total_waiters );
        }
    }

protected :
    void Wake ( int count )
    {
        INTERLOCKED_WRITE_RELEASE(&total_waiters,total_waiters - count );
        ::ReleaseSemaphore ( semaphore , count , NULL );
    }

private :
    HANDLE semaphore;
    long    total_waiters;
};

Ответы [ 4 ]

2 голосов
/ 27 января 2010

Я думаю, что плохие вещи произойдут, если вы скопируете свой экземпляр, поскольку обе копии будут использовать один и тот же семпахор. Это не обязательно плохо, но это может смутить людей, если семантика не совсем ясна.

Вы можете легко исправить это аналогично тому, что boost :: noncopyable использует (или использует boost).

1 голос
/ 27 января 2010

Я предпочитаю реализацию Boost wait(), потому что требуется объект блокировки RAII для обеспечения синхронизации доступа к общему состоянию условия. RAII облегчает написание исключительного кода.

Я аннотировал найденный пример кода здесь : 

boost::condition_variable cond;
boost::mutex mut;
bool data_ready;

void process_data();

void wait_for_data_to_process()
{
    boost::unique_lock<boost::mutex> lock(mut);  // Mutex acquired here (RAII).
    while(!data_ready) // Access to data_ready is sync'ed via mutex 'mut'
    {
        // While we are blocking, the mutex is released so
        // that another thread may acquire it to modify data_ready.
        cond.wait(lock);

        // Mutex is acquired again upon exiting cond.wait(lock)
    }
    process_data();

    // Mutex is released when lock goes out of scope.
}
0 голосов
/ 27 января 2010

(самостоятельный ответ)

Я нашел большую ошибку.

У CondVars есть внешняя блокировка ... и это не так.

0 голосов
/ 27 января 2010

если вы используете функции WinAPI, вероятно, лучше использовать InterlockedIncrement(...) и InterlockedDecrement(...) вместо INTERLOCKED_WRITE_RELEASE(&total_waiters,total_waiters + 1 ); и INTERLOCKED_WRITE_RELEASE(&total_waiters,total_waiters - count ); соответственно.

...