2010-01-27 4 views
2

Je suis nouveau dans la synchronisation de threads. Je lisais de nombreuses implémentations de variables conditionnelles, comme boost :: threads et pthread pour win32. J'ai juste implémenté ce moniteur assez simple avec wait/notify/noifyall, et je suppose qu'il y a beaucoup de problèmes cachés, que j'aimerais découvrir chez des gens plus expérimentés. Toute suggestion?Qu'est-ce qui est dangereux dans ce fil extrêmement simple Mise en œuvre du moniteur?

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

cela utilise la bibliothèque boost? – lsalamon

+0

bien, je viens de copier les macros INTERLOCKED_READ_ACQUIRE/INTERLOCKED_WRITE_RELEASE, pour lire/écrire les compteurs en mémoire en utilisant des barrières de mémoire. –

+0

@Isalamon: Je pense qu'il veut rouler sa propre classe de condvar en utilisant Boost.Threads pour l'inspiration. –

Répondre

0

si vous utilisez des fonctions WinAPI, il est probablement préférable d'utiliser InterlockedIncrement(...) et InterlockedDecrement(...) au lieu de INTERLOCKED_WRITE_RELEASE(&total_waiters,total_waiters + 1); et INTERLOCKED_WRITE_RELEASE(&total_waiters,total_waiters - count); respectivement.

+0

c'est seulement une macro ... cela signifie une opération atomique pour augmenter/diminuer la variable –

+0

Mais il est inutilement compliqué. Si vous êtes lié à l'API Win32 de toute façon, pourquoi ne pas utiliser les opérations atomiques fournies ici? D'autres programmeurs Win32 les connaissent déjà. – jalf

2

Je pense que de mauvaises choses se produiront si vous copiez votre instance puisque les deux copies utiliseront le même sempahore. Ce n'est pas nécessairement une mauvaise chose, mais cela risque de dérouter les gens si la sémantique n'est pas entièrement claire.

Vous pouvez facilement corriger cela avec une approche similaire à ce que boost::noncopyable utilise (ou utiliser boost).

+0

oui! Je vous remercie. –

1

Je préfère l'implémentation de Boost de wait(), car il prend un objet verrou RAII pour s'assurer que l'accès à l'état de condition partagé est synchronisé. RAII facilite l'écriture de code de sécurité exceptionnelle.

J'ai annoté l'exemple de code trouvé 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

(auto réponse)

j'ai trouvé une grosse erreur. CondVars ont un verrou externe ... et cela n'a pas.