sdt::queue und std::list nicht Thread-Safe?



  • Hi,

    ich habe eine Applikation, welche mir eine Queue (alternativ have ich auch eine List probiert) mit Daten füllt (push() bzw. push_back()) während ein separater Thread diese wieder abholt (pop_front() bzw. pop()). Die Zugriffe auf die List bzw. die Queue sind per Mutex abgesichert.

    Unter Linux (Thread und Mutex per pthreads) läuft das Ganze problemlos. Unter Windows mit VC++6.0 und CreateThread bzw. dem Win-API-Mutex funktioniert der gleiche Code gar nicht. Hier fülle ich zwar Daten in die Queue, innerhalb des Threads bleibt diese aber leer (empty()==true).

    Was kann da los sein? Ist die Windows-Implementierung etwa nicht Thread-Safe??



  • Bist du überhaupt sicher, daß beide Threads mit der selben list/queue arbeiten? Nicht daß du versehentlich eine Kopie angelegt hast und deshalb nichts siehst.

    (Probleme mit der Thread-Sicherheit würden afaik sich anders bemerkbar machen)



  • Dikdik schrieb:

    Die Zugriffe auf die List bzw. die Queue sind per Mutex abgesichert.
    (...)
    Was kann da los sein? Ist die Windows-Implementierung etwa nicht Thread-Safe??

    Wenn die Zugriffe wirklich korrekt über ne Mutex abgesichert sind, dann muss die Implementierung nur insofern thread-safe sein, als dass sie keine globalen/static Variablen verwendet, bzw. den Zugriff auf diese selbst synchronisiert (sonst könnte man nicht verschiedene Objekte zur gleichen Zeit in verschiedenen Threads verwenden).

    Und die STL Implementierung von MSVC ist in diesem Sinne thread safe (verschiedene Objekte in verschiedenen Threads = OK, alles andere = selber drum kümmern).
    (Zumindest ab MSVC Version 6.0, zu älteren Versionen kann ich nichts sagen, hab ich nie verwendet)

    Der Fehler ist mit an Sicherheit grenzender Wahrscheinlichkeit in deinem Code zu finden. D.h. du solltest den mal herzeigen, sonst können wir schwer helfen.

    BTW: das was Windows als MUTEX ( CreateMutex() , ReleaseMutex() ) bezeichnet ist fast immer Overkill (viel zu langsam). Verwende CRITICAL_SECTION .
    Und es ist ratsam unter Windows _beginthreadex zum Erzeugen von Threads zu verwenden. Meistens funktioniert es zwar auch mit CreateThread problemlos, aber wenn man auf der sicheren Seite sein will, sollten man _beginthreadex verwenden.



  • hustbaer schrieb:

    Und es ist ratsam unter Windows _beginthreadex zum Erzeugen von Threads zu verwenden. Meistens funktioniert es zwar auch mit CreateThread problemlos, aber wenn man auf der sicheren Seite sein will, sollten man _beginthreadex verwenden.

    Wobei hier noch erwähnt werden sollte, dass CreateThread einen Memory Leak von ein paar Bytes erzeugt.



  • > Bist du überhaupt sicher, daß beide Threads mit der selben list/queue arbeiten?

    Ja, die Adressen habe ich verglichen, die passen.

    Mein Hauptproblem: der identische Code (abgesehen von den Systemabhängigen Thread- und Mutex-Geschichten) läuft unter Linux (mit dem GCC compiliert) problemlos.

    Die Queue erzeuge ich mir übrigens per

    m_ctrlList=new std::queue<struct oapc_bin_head*>;
    

    ...aber das sollte unkritisch sein!?



  • 314159265358979 schrieb:

    hustbaer schrieb:

    Und es ist ratsam unter Windows _beginthreadex zum Erzeugen von Threads zu verwenden. Meistens funktioniert es zwar auch mit CreateThread problemlos, aber wenn man auf der sicheren Seite sein will, sollten man _beginthreadex verwenden.

    Wobei hier noch erwähnt werden sollte, dass CreateThread einen Memory Leak von ein paar Bytes erzeugt.

    Ja, genau darum geht es, das ist ja der Grund warum man _beginthreadex verwenden sollte.
    Wobei "erzeugt" falsch ist: erzeugen kann.
    Es gibt genug Situationen wo es (garantierterweise) nicht passiert.



  • @Dikdik:
    Zeig einfach mal ein paar Codestellen in der Windows Variante.
    Also z.B. Setup (Erzeugen der Objekte, starten der Threads) sowie Schreiben und Lesen der Queue.



  • Dikdik schrieb:

    ...aber das sollte unkritisch sein!?

    Du muss einfach jeden Zugriff auf gemeinsam genutzte Daten absichern. z.B:

    class myqueue
    {
      static std::queue<> m_tasks;
    public:
      static bool pushTask(const Task& task)
      {
        getmutex(mutex);
        m_tasks.push(task); 
        releasemutex(mutex);
        return true;
      }
      bool popTask(Task& task)
      {
        getmutex(mutex);
        task = m_tasks.pop();
        releasemutex(mutex);
        return true;
      }
    };
    
    int main()
    {
      my_queue::pushTask(x);
      my_queue::pushTask(y);
      beginthread(my_queue()); // neue Instanz für jeden Thread!, hier darf nur pop() aufgerufen werden.
      my_queue::pushTask(z); // gültig, da m_tasks gesichert ist.
    }
    


  • cooky451 schrieb:

    static std::queue<> m_tasks;
    

    Weswegen ein statischer queue?

    cooky451 schrieb:

    beginthread(my_queue()); // neue Instanz für jeden Thread!, hier darf nur pop()
    

    Warum eine Instanz für jeden Thread?
    Und warum darf nur pop() aufgerufen werden, wenn über den gleichen Mutex geschützt wird.

    Ich denke eine Implementierung ala

    template<typename T>
    class sync_queue{
    public:
            void push(const T& t){
                    boost::mutex::scoped_lock l(mutex);
                    queue.push(t);
            }
    
            T pop(){
                    boost::mutex::scoped_lock lock(mutex);
                    T t = queue.front();
                    queue.pop();
                    return t;
            }
    
    private:
            boost::mutex mutex;
            std::queue<T> queue;
    };
    

    ist wesentlich eleganter, weil man mehrere queues haben kann...

    Oder analog eine blockende Variante:

    template<typename T>
    class blocking_sync_queue{
    public:
            void push(const T& t){
                    boost::mutex::scoped_lock l(mutex);
                    queue.push(t);
                    empty_condition.notify_one();
            }
    
            T pop(){
                    boost::mutex::scoped_lock lock(mutex);
                    while(queue.empty())
                            empty_condition.wait(lock);
                    T t = queue.front();
                    queue.pop();
                    return t;
            }
    
    private:
            boost::mutex mutex;
            std::queue<T> queue;
            boost::condition_variable empty_condition;
    };
    

    Gruß,
    XSpille


Log in to reply