3. Alternativer Ansatz gesucht - Verbindungsauf-/-abbau und Aufrechterhaltung

Alternativer Ansatz gesucht - Verbindungsauf-/-abbau und Aufrechterhaltung

  • G

    Hallo zusammen,
    ich stehe wie ein Ochs vorm Berg. Meine bisherige Lösung gefällt mir nicht (wirkt für mich wie Bastelarbeit), ich suche daher "schöne" Alternativen. (schön = lesbar, wartbar, effizient - die eierlegende Wollmichsau halt 😉 )

    Es sei die Klasse:

    /**Verbindet was.
*/
class Connector
{
    std::thread* m_tHandle;
    std::atomic<bool> m_bKeepAlive;

    /**Hält eine Verbindung aufrecht.
    */
    void keepAlive();

public:
    /**Stellt eine Verbindung her.
    */
    int connect();

    /**Beendet eine bestehende Verbindung.
    */
    int disconnect();

    /**Führt eine Interaktion.
    */
    int interact();
};

    Die Definition sähe dann irgendwie so aus (Pseudo)

    int Connector::connect()
{
    // Fehlerfall
    if( gibtSchonEineVerbindung() )
    {
        return 1;
    }

    // Verbindung herstellen
    try
    {
        /* Hier Kram, der die Verbindung herstellt */

        // Verbindung aufrecht halten
        m_bKeepAlive = true;
        m_tHandle = new std::thread( keepAlive );
    }
    catch(...)
    {
        return E_FEHLERCODE;
    }

    return 0;
}

int Connector::keepAlive()
{
    while( m_bKeepAlive )
    {
        /* Hier Kram, der die Verbindung aufrecht hält. */
    }

    return 0;
}

int Connector::disconnect()
{
    // Fehlerfall
    if( gibtGarKeineVerbindung() )
    {
        return 1;
    }

    // Verbindung trennen
    try
    {
        // Verbindung nicht länger aufrecht halten
        m_bKeepAlive = false;
        m_tHandle->join();

        /* Hier Kram, der die Verbindung trennt. */
    }
    catch(...)
    {
        return E_FEHLERCODE;
    }

    return 0;
}

int Connector::interact()
{
    /* Interaktion */
}

    So, ich hoffe, nichts Wesentliches vergessen zu haben. Das Ganze funktioniert jedenfalls grundsätzlich.

    Zum Problem:
    Der Urheber scheint den Gedanken verfolgt zu haben, mit connect und disconnect zwei öffentliche Funktionen zum Verbindungsauf- und -abbau zur Verfügung zu stellen. Vom Wiederaufbauen der Verbindung im Falle einer Störung sollte der Nutzer offenbar befreit bleiben. Darum kümmert sich stattdessen die private Funktion keepAlive . Diese wird bei Verbindungsaufbau gestartet und läuft bis sie durch den Verbindungsabbau gestoppt wird.

    So weit, so gut. Nun gibt es leider ein Problem mit der Funktion keepAlive . Diese schafft es - entgegen ihres gewählten Namens - eben nicht immer die Verbindung aufrecht zu erhalten. Tritt dieser Fall ein, führt die nächste Interaktion mit der Gegenstelle zum Absturz des Programms, weil auf eine Ressource zugegriffen wird, die nicht mehr verfügbar ist.

    Daher möchte ich keepAlive gern umstricken und mein erster Gedanke ging in folgende Richtung:

    int Connector::keepAlive()
{
    while( m_bKeepAlive )
    {
        while( m_bKeepAlive && verbindungBesteht() )
        {
            /* Hier Kram, der die Verbindung aufrecht hält. */
        }

        // Nanu, warum sind wir denn hier?
        if( !m_bKeepAlive )
        {
            // Ist Absicht, jemand hat disconnect() aufgerufen.
        }
        else
        {
            // Verbindung abgeraucht ==> neu aufbauen
            disconnect();  // Verbindung sauber trennen
            connect();     // Verbindung sauber neu aufbauen
        }
    }

    return 0;
}

    Das war zu einfach gedacht: disconnect kann nicht innerhalb von keepAlive aufgerufen werden, da disconnect den Thread beendete, in dem keepAlive liefe. Das Programm haute sich selbst den Boden unter den Füen weg.

    Mein zweiter Ansatz sieht derzeit so aus, dass ich zwei zusätzliche Funktionen habe, in die der Verbindungsauf- und -abbau ausgelagert wurden. Diese werden anstelle von connect und disconnect in keepAlive aufgerufen. Im Prinzip also:

    int Connector::connect()
{
    // ausgelagerter Verbindungsaufbau
    baueVerbindungAuf();

    // Verbindung aufrecht halten
    m_bKeepAlive = true;
    m_tHandle = new std::thread( keepAlive );
}

int Connector::disconnect()
{
    // Verbindung nicht länger aufrecht halten
    m_bKeepAlive = false;
    m_tHandle->join();

    // Ausgelagerter Verbindungsabbau
    beendeVerbindung();
}

int Connector::keepAlive()
{
    while( m_bKeepAlive )
    {
        while( m_bKeepAlive && verbindungBesteht() )
        {
            /* Hier Kram, der die Verbindung aufrecht hält. */
        }

        // Nanu, warum sind wir denn hier?
        if( !m_bKeepAlive )
        {
            // Ist Absicht, jemand hat disconnect() aufgerufen.
        }
        else
        {
            // Verbindung abgeraucht ==> neu aufbauen
            beendeVerbindung();   // Verbindung sauber trennen
            baueVerbindungAuf();  // Verbindung sauber neu aufbauen
        }
    }

    return 0;
}

    Das scheint zu laufen, aber schön finde ich den Weg nicht. Das sieht so rumgebastelt aus. Daher die Frage nach Alternativen in die Runde: Geht das schöner? Wenn ja, wie?

    Und jetzt schon mal vielen Dank fürs Lesen. 👍

    Viele Grüße
    gelignite

    0

    1. Dir fehlt vermutlich (=wenn du es hier nicht unterschlagen hast) Synchronisierung
    2. Bereits minimales Refactoring führt zu einer deutlichen Verbesserung des "unschön" Faktors:
    int Connector::keepAliveThread()
{
    while (m_bKeepAlive)
        keepAliveTick();

    // Ist Absicht, jemand hat disconnect() aufgerufen.
    cleanUpStuff();
}

void Connector::keepAliveTick()
{
    if (!verbindungBesteht())
        recover();

    /* Hier Kram, der die Verbindung aufrecht hält. */
}

void Connector::recover()
{
    // Verbindung abgeraucht ==> neu aufbauen
    beendeVerbindung();   // Verbindung sauber trennen
    baueVerbindungAuf();  // Verbindung sauber neu aufbauen
}
    1. Überleg dir ob es nicht sinn machen würde den "Connector" in zwei Klassen zu trennen: 1x die Klasse die die ganzen Operationen der Verbindung implementiert/kapselt und dann 1x eine Klasse die für den Keep-Alive-Thread und den automatischen recover/reconnect zuständig ist.
    0
  • R

    wenn die verbindung einzig wegen "interact" noetig ist, wozu dann der thread und das staendige am leben halten? kann "interact" nicht einfach selbst sowas regeln ala

    int Connector::interact()
{
    if(!verbindungBesteht())
        reconnect();
    /* Interaktion */
}

    ?

    so off topic: ich faende es sauberer wenn deine funktionen wirklich error types returnen und nicht int mit mal 0, 1, E...

    0
  • G

    Hallo,

    rapso schrieb:

    wenn die verbindung einzig wegen "interact" noetig ist, wozu dann der thread und das staendige am leben halten?

    so off topic: ich faende es sauberer wenn deine funktionen wirklich error types returnen und nicht int mit mal 0, 1, E...

    Es wird immer gern ein Minimalbeispiel gefordert, das das Problem zeigt, deshalb steht interact hier mehr oder weniger symbolisch. Es gibt weitere Funktionen, die ihrerseits mit der Ressource kommunizieren.
    Das Projekt war einerseits wohl mal ein Studentenprojekt, andererseits war es wohl auch mal in C verfasst (oder von einem C-ler geschrieben). Zumindest bekommt man den Eindruck. Es steht überall int als Rückgabetyp und nicht z.B. bool oder ein Enumerator. Was ich als E_ ... gekennzeichnet habe, ist auch nicht wirklich ein enum . Stattdessen ist man hergegangen und hat die Werte mittels #define angelegt.
    Ich will gar nicht wissen, was hier noch für Leichen im Keller liegen.

    hustbaer schrieb:

    1. Dir fehlt vermutlich (=wenn du es hier nicht unterschlagen hast) Synchronisierung
    2. Bereits minimales Refactoring führt zu einer deutlichen Verbesserung des "unschön" Faktors:
    3. Überleg dir ob es nicht sinn machen würde den "Connector" in zwei Klassen zu trennen: 1x die Klasse die die ganzen Operationen der Verbindung implementiert/kapselt und dann 1x eine Klasse die für den Keep-Alive-Thread und den automatischen recover/reconnect zuständig ist.
    1. Mir fehlt vor allem der Überblick, was in diesem Code noch so alles vor sich geht. Es gibt - wer hätte das gedacht - keine Dokumentation. Vereinzelte Funktionen besitzen einen Kommentar in der Form (Name, Beschreibung, Eingabeparameter, Rückgabeparameter). Aber lass das mal 10 % der Funktionen sein, auf die das zutrifft.
    2. Probier ich.
    3. Das ist ein altes, sterbendes Projekt. Zu viel Aufwand werde ich da sicher nicht mehr investieren. Den vorliegenden Fehler will ich halt nur noch möglichst sauber lösen. Um ggf. was für zukünftige Dinge mitzunehmen, schaue ich mir 2) auf jeden Fall noch an.

    Danke und einen schönen Tag euch.

    Viele Grüße
    gelignite

    0
Anmelden zum Antworten