2010-01-27 1 views
2

Я новичок в синхронизации потоков. Я читал много реализаций условных переменных, таких как 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; 
}; 
+0

это использует библиотеку ускорения? – lsalamon

+0

ну, я просто скопировал макросы INTERLOCKED_READ_ACQUIRE/INTERLOCKED_WRITE_RELEASE, чтобы читать/записывать счетчики в память с помощью заграждений памяти. –

+0

@ Isalamon: Я думаю, он хочет катить свой собственный класс condvar, используя Boost.Threads для вдохновения. –

ответ

0

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

+0

это только макрос ... это означает атомную операцию для увеличения/уменьшения переменной –

+0

Но она излишне запутана. Если вы все равно привязаны к API Win32, почему бы не использовать атомные операции, предусмотренные там? Другие программисты Win32 будут уже знакомы с ними. – jalf

2

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

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

+0

да! Спасибо. –

1

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

Я аннотированный пример кода найден here:

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

(самообеспечение ответ)

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

У CondVars есть внешний замок ... и это не имеет.