Kritische Abschnitt - sein oder nicht sein?

Question

I`m einen Chat mit WinSock2 und WinAPI Funktionen zu schreiben. Und ich habe ein wenig Mühe.
Ich speichere die std :: vector von Client-Verbindungen auf dem Server. Wenn neue Client eine Verbindung herstellt, neuen Thread startet und alle Arbeiten mit dem Kunden wird in diesem neuen Thread getan. Ich benutze keine Klassen (Ich weiß, es ist nicht sehr gut) so dass diese Liste der Verbindungen wird nur als globale Variable definiert.
Es scheint mir, dass es eine Situation sein kann, wenn mehrere Threads gleichzeitig versuchen, diese Liste zuzugreifen. Obwohl ich nicht bemerkt habe, dass es irgendwelche Probleme damit, brauche ich so etwas wie dies zu tun:

template 
class SharedVector {
    std::vector vect;
    CRITICAL_SECTION cs;
    SharedVector(const SharedVector& rhs) {}
public:
    SharedVector();
    explicit SharedVector(const CRITICAL_SECTION& CS);
    void PushBack(const T& value);
    void PopBack();
    unsigned int size();
    T& operator[](int index);
    virtual ~SharedVector();
};

template
SharedVector::SharedVector() {
    InitializeCriticalSection(&cs);
}

template
SharedVector::SharedVector(const CRITICAL_SECTION& r): cs(r) {
    InitializeCriticalSection(&cs);
}

template
void SharedVector::PushBack(const T& value) {
    EnterCriticalSection(&cs);
    vect.push_back(value);
    LeaveCriticalSection(&cs);
}

template
void SharedVector::PopBack() {
    EnterCriticalSection(&cs);
    vect.pop_back();
    LeaveCriticalSection(&cs);
}

Also, tut meine Situation erfordert mit CRITICAL_SECTION und bin ich nur der glückliche Kerl, der keinen Fehler gefunden?

Answer 1

Ja, Sie sind einfach Glück nie irgendwelche Probleme auftreten. Das ist das Problem mit Synchronisierungsproblemen und Rennbedingungen, dass der Code in 99,9% aller Fälle funktioniert, und im Katastrophenfall werden Sie nicht wissen, warum.

Ich würde den Konstruktor nimmt eine CRITICAL_SECTION als Parameter weg, da sie, ohne auf (vermutlich nicht existente) Dokumentation nicht klar zu erkennen, dass der Konstruktor initialisieren wird.

Answer 2

Dieser Code ist nicht Ausnahme sicher, Vektor push_back und pop_back Methoden können Ausnahmen werfen, und Sie haben hier potentielle Deadlock. Diese Methoden:

unsigned int size();
T& operator[](int index);

muss auch synchronisiert werden, weil sie auf Daten zugreifen kann, dass von einem anderen Thread geändert werden. Zum Beispiel können Sie Daten wie folgt zugreifen:

value = shared_vector[shared_vector.size() - 1];

und zur gleichen Zeit, ein anderer Thread kann dies tun:

shared_vector.PopBack();

Answer 3

Die größte Frage, die ich für Sie ist architektonisch. Ist der Thread für jede Verbindung wirklich braucht direkten Zugang zu dieser Reihe von andere Verbindungen? Sollte nicht wirklich sie abstrakt Nachrichten von einer Art sein Einreihen? Wenn jedes Anschlussgewinde der Implementierungsdetails des Universums bewusst ist, in der er lebt, erwarte ich, dass Sie in Schwierigkeiten irgendwo auf der ganzen Linie laufen werden.

Aber nehmen wir an der Anschlussgewinde direkt tun müssen wirklich Zugang zueinander ...

Globale Variablen sind nicht von Natur aus böse; Schulen lehren nur, weil es einfacher ist als ein differenziertes Verständnis bereitstellt. Sie haben die genauen Auswirkungen einer globalen Variablen kennen, und es ist ein anständiger Faustregel anzunehmen, dass ein global die falsche Wahl ist, bis Sie entdecken Sie keine Alternative haben. Eine Alternative, die eine Reihe von Problemen löst, ist eine Funktion wie folgt zu schreiben:

SharedVector & GetSharedVector (void)
{
    static SharedVector sharedVector;
    return sharedVector;
}

Das kauft Ihnen mehr Kontrolle über, wenn die Klasse instanziiert wird, als Sie mit einer globalen Variablen haben würde, die entscheidend sein können, wenn Sie Abhängigkeiten zwischen globalen Variablen haben die Konstrukteure haben, vor allem, wenn diese globalen Variablen Threads erzeugen. Es gibt Ihnen auch die Möglichkeit, zu intervenieren, wenn jemand Zugriff auf diesen „global“ Variable will nicht nur ohnmächtig leiden, während sie es direkt stecken.

Eine Reihe von anderen Gedanken entspringt auch in dem Sinne. In keiner bestimmten Reihenfolge ...

• Sie ohne Zweifel bereits kennen die Template Syntax Sie hier verwenden ist seltsam; Ich nehme an, den Sie gerade eingegeben dieser Code in, ohne es zu kompilieren.
• Sie benötigen eine do-nothing Zuordnung Betreiber in Ihrem privaten Bereich zu Vermeidung von Unfällen (für die gleichen Grund haben Sie eine do-nothing Kopie Konstruktor dort).
• Die explizite Copykonstruktor ist in einer Reihe von Möglichkeiten gebrochen. Es braucht explizit den Vektor zu kopieren oder Sie werden mit einem leeren Vektor in der neuen Instanz von SharedVector enden. Und es sollte einfach initialisieren seine eigener kritische Abschnitt statt kopieren (und initialisieren es dann). Wirklich für Ihren Fall ist es wahrscheinlich macht keinen Sinn, diese zu haben Funktion überhaupt, weil Netzwerk Verbindungen müssen nicht zumutbar Kopie Semantik.
• Sie können Exception-Sicherheit erleichtern durch Erstellen einer Klasse EnteredCriticalSection deren Konstruktoraufrufe Und dessen Entercriticalsection destructor Anrufe Leavecriticalsection. (Es muss Halten auf eine Bezugnahme auf die Diese kritische Abschnitt, natürlich.) macht es zu einer Tonne leichter sicher serialisiert Ihr anderes Mitglied Funktionen angesichts der Ausnahmen.

Answer 4

Ja, wenn Sie einen globalen Vektor wie folgt aussetzen, müssen Sie definitelly es sperren, auf jedem Lese / Schreib. Und ja, dass Ihre Leistung schlecht verletzen können.

Auch ein neuer Thread pro Anfrage ist in der Regel nicht eine sehr gute Idee. Warum gehst du nicht Redesign Ihre Anwendung Ports IO Completion stattdessen verwenden?

Answer 5

Und übrigens, es ist kein guter Weg, diesen Vektor als globale Variable zu deklarieren. Wird es zu machte es lokale ein besser sein?