C++ Sockets unter Ubuntu.

mikemodanoxxx

Mein Puffer ist so groß wie ein Vektor, es sind aber schon zwei gesendet worden.

Eisflamme

Wieso versendest Du statt "2x Länge - Vektor1 - Vektor2" nicht "Länge + Vektor1" und "Länge + Vektor2"? Dann hättest Du das Problem nicht.

tntnet

Wenn Du deine "Empfange()"-Funktion richtig implementierst gibt es keinen Pufferüberlauf. Deine Vorgehensweise entspricht in etwa der einfachsten vorgeschlagenen Lösung. Also ein Längenwort (bzw. bei Dir 2) und die Daten hinterher. Und wenn der Client zuerst die Länge liest und dann genau so viele Bytes, wie die Länge angibt, dann funktioniert das. Wenn Du dabei einen Pufferüberlauf bekommst, hast Du es einfach nur falsch implementiert.

Ich empfehle eine Pufferungsschicht einzubauen. Wenn Du dann das Längenwort liest, dann liest die Pufferungsschicht so viel Daten, wie am Socket anliegen und liefert nur das angeforderte Längenwort. Der nächste Aufruf wird dann aus den bereits gelesenen Daten versorgt.

Das gleiche gilt für die Senderseite. Du fragst Dich, ob Du erst die Daten zurecht legen sollst und dann senden oder stückchenweise senden? Pufferung löst das ganze. Die obere Schicht schreibt ein Längenwort (bzw. 2 bei Dir) in den Puffer und dann übergibt es die Daten. Wenn es fertig ist, dann sagt sie der Pufferungsschicht, dass er fertig ist. Das nennt man in der regel "flush". Die PUfferungsschicht hat eine feste Puffergrösse, wie beispielsweise 8k. Immer wenn sie voll wird, wird geschickt. Und wenn dann ein flush kommt, dann wird der Rest geschickt.

Der Vorteil ist, dass die Applikationsschicht die Daten logisch so verarbeiten kann, wie es sinnvoll erscheint. Byteweises lesen beispielsweise ist dann kein Problem, da die Pufferungsschicht dieses ineffiziente byteweise lesen in größere Blöcke übersetzt.

Aber das ganze musst Du nur beachten, wenn Du wirklich alles zu Fuß programmieren willst. Besser ist es, fertige Module zu verwenden. Eventuell mit einer höheren Abstraktionsschicht. Wie gesagt verwenden wir eine Bibliothek, die vieles abnimmt. Da machen wir uns in der Applikation keine Gedanken mehr, ob wir ein Längenbyte schicken, sondern wir schicken einfach Datenstrukturen. Die Bibliothek hat sich um das Protokoll zu kümmern. Auch diese ganze Pufferung geschieht dort in den unteren Ebenen.

knivil

Wieso versendest Du statt "2x Länge - Vektor1 - Vektor2" nicht "Länge + Vektor1" und "Länge + Vektor2"? Dann hättest Du das Problem nicht.

Das ist doch voellig egal. Es darf halt nur soviel ausgelesen werden wie Vector1 lang ist.

Wie wird gesendet und empfangen (Code bitte)?

mikemodanoxxx

Habe es jetzt umgeschrieben und er läuft wesentlich besser. Ab und zu bleibt er trotzdem noch hängen. Vielleicht findet ja jemand den Fehler oder sagt mir wie ich das Programm verbessern kann. Als Socket-Typ benutze ich AF_LOCAL.

Funktion zum Senden:

bool Socket::send(SocketMsg msg) const {
    // check size
    int size[5];
    size[0] = msg.functionType;
    size[1] = msg.doubleIO.size();
    size[2] = msg.doubleIO2.size();
    size[3] = msg.int64IO.size();
    size[4] = msg.boolIO;
    // Send size information
    int status = ::send(m_sock, size, sizeof(int)*5, 0);
    if(status == -1) {
        return false;
    }
    // Send first double vector
    status = ::send(m_sock, &msg.doubleIO[0], sizeof(double)*size[1], 0);
    if(status == -1) {
        return false;
    }
    // Send second double vector
    status = ::send(m_sock, &msg.doubleIO2[0], sizeof(double)*size[2], 0);
    if(status == -1) {
        return false;
    }
    // Send uint64_t vector
    status = ::send(m_sock, &msg.int64IO[0], sizeof(uint64_t)*size[3], 0);
    if(status == -1) {
        return false;
    }

    return true;
}

Empfangen:

SocketMsg Socket::recv() const {
    SocketMsg msg;

    int size[5];

    // Receive function type
    int status = ::recv(m_sock, size, sizeof(int)*5, 0);

    if (status == -1) {
        std::cout << "status == -1   errno == " << errno << "  in Socket::recv\n";
        return msg;
    } else if(status == 0) {
        std::cout << "nothing received" << std::endl;
        return msg;
    }

    // initialize message
    msg.functionType = static_cast<FunctionType>(size[0]);
    msg.doubleIO = std::vector<double>(size[1]);
    msg.doubleIO2 = std::vector<double>(size[2]);
    msg.int64IO = std::vector<uint64_t>(size[3]);
    msg.boolIO = size[4];

    // calculate total packet size
    int packetSize = sizeof(double)*size[1]
            + sizeof(double)*size[2]
            + sizeof(uint64_t)*size[3];

    char buffer[5000];
    int received = 0;

    while (received < packetSize) {
        status = ::recv(m_sock, &buffer[received], packetSize, 0);
        if (status == -1) {
            std::cout << "status == -1   errno == " << errno << "  in Socket::recv\n";
            msg.functionType = Undefined;
            return msg;
        }
        received += status;
    }

    // reassamble message
    memcpy(&msg.doubleIO[0], &buffer[0], sizeof(double)*size[1]);
    memcpy(&msg.doubleIO2[0], &buffer[sizeof(double)*size[1]], sizeof(double)*size[2]);
    memcpy(&msg.int64IO[0], &buffer[sizeof(double)*size[1]+sizeof(double)*size[2]], sizeof(uint64_t)*size[3]);
    return msg;
}

Die zwei Funktionen sollen von Sender und Empfänger quasi immer abwechselnd aufgerufen werden. Sieht da jemand einen Fehler? Ansonsten müsste ich ihn an einer anderen Stelle suchen.

tntnet

So spontan fällt mir erst mal auf, dass Du mit delete einen Puffer löschst, der auf dem Stack liegt. Außerdem prüfst Du beim ersten recv nicht, ob die Daten vollständig sind. Es könnte sein, dass Du nur ein Teil empfängst.

Auf der Senderseite könnte das gleiche passieren. send könnte nur einen Teil der Nachricht senden. Dann geht dein Protokoll kaputt.

Vielleicht hat Du meinen vorigen Kommentar gelesen. Umgesetzt hast Du das nicht.

Es ist leichter und ganz automatisch aus effizienter, wenn Du verschiedene Schichten programmierst. Die Applikationsschicht schickt einfach nur vektoren. Die mittlere Protokollschicht fügt die notwendigen Protokollinformationen wie die Vektorlängen und so hinzu. Die Netzwerkschicht empfängt einfach nur Bytes und verschickt sie und kümmert sich um die Pufferung. Und auf der anderen Seite genauso.

Architektonisch ist es falsch, wenn Deine Klasse mit dem Namen "Socket" irgendetwas über "functionType" oder so was weiß. Socketkommunikation ist schwer genug.

Kleinigkeiten, die Du nicht berücksichtigt hast sind auch so was wie EINTR. Ein Systemaufruf könnte immer durch ein Signal unterbrochen werden. In der Regel sollte der Aufruf wiederholt werden.

Und nochmal: Nimm eine fertige Library. Es ist echt einfacher. Unter Ubuntu findest Du in den Repositories beispielsweise boost oder mein cxxtools. Poco ist wohl auch dabei.

knivil

http://beej.us/guide/bgnet/

mikemodanoxxx

Ja das mit dem delete kannst du ignorieren. Ich hatte hier kurz was im Code geändert und es vergessen hier im Formular anzupassen.

Ich hatte am Anfang mal überlegt boost zu verwenden, aber dann gelesen, dass es für so kleine Probleme wohl ziemlich überdimensioniert sei. Dann begebe ich mich mal auf die Suche nach ner gescheiten Library..

cooky451

Warum bitte schreibst du erst in einen Puffer auf dem Stack, nur um dann die Daten zu kopieren? Hä?

mikemodanoxxx

Also ich habe dann doch noch meine Lösung etwas weiterentwickelt und sie funktioniert jetzt soweit. Sie muss nicht perfekt sein, ich brauche sie nur um mein Projekt zu evaluieren. Wenn das ohne Crash durchläuft ist es in Ordnung :). Es war sowieso nur eine Notlösung weil ich zwei Anwendungen nicht verknüpfen konnte und es daher über IPC machen musste.

Und @tntnet: Ja ich stimme dir zu, dass der Socket den allgemeinen Datentyp nutzen sollte (wie hier jetzt nur über den Char-Pointer). Es erben allerdings SocketClient und SocketServer von dieser Klasse und ich fand es daher "einfacher" das so zu machen. Jetzt gibt es ja wenigstens noch zusätzlich die allgemeinere Schnittstelle :). Der Hinweis, dass der Sendevorgang unterbrochen werden kann hat auch sehr geholfen! Ich bin eigentlich davon ausgegangen, dass so eine Systemfunktion ein atomarer Vorgang ist (also ich dachte er kann vom BS unterbrochen werden, wird dann aber fortgesetzt und wird nach außen ganz normal beendet).

Jedenfalls danke an alle für eure Hilfe.

Hier ist mal der Code:

bool Socket::send(char* dataPointer, int dataSize) const {
    int sent = 0;

    while(sent < dataSize) {
        int status = ::send(m_sock, dataPointer+sent, dataSize-sent, 0);

        if(status == -1 && status != EINTR) {
            return false;
        }
        sent += status;
    }

    return true;
}

bool Socket::send(SocketMsg msg) const {
    int size[5];
    size[0] = msg.functionType;
    size[1] = msg.doubleIO.size();
    size[2] = msg.doubleIO2.size();
    size[3] = msg.int64IO.size();
    size[4] = msg.boolIO;

    bool success = true;

    send(reinterpret_cast<char*>(&size[0]), sizeof(int)*5) ? true : false;
    send(reinterpret_cast<char*>(&msg.doubleIO[0]), sizeof(double)*size[1]);
    send(reinterpret_cast<char*>(&msg.doubleIO2[0]), sizeof(double)*size[2]);
    send(reinterpret_cast<char*>(&msg.int64IO[0]), sizeof(uint64_t)*size[3]);

    // TODO: success flag
    return success;
}

int Socket::recv(char* buffer, int size) const {
    int received = 0;

    while(received < size) {
        int status = ::recv(m_sock, &buffer[received], size-received, 0);
        if (status == -1 && status != EINTR) {
            std::cout << "status == -1   errno == " << errno << "  in Socket::recv\n";
            return -1;
        }
        received += status;
    }

    return received;
}

SocketMsg Socket::recv() const {
    SocketMsg msg;

    int size[5];

    // Receive function type
    recv(reinterpret_cast<char*>(&size[0]), 5*sizeof(int));

    // initialize message
    msg.functionType = static_cast<FunctionType>(size[0]);
    msg.doubleIO = std::vector<double>(size[1]);
    msg.doubleIO2 = std::vector<double>(size[2]);
    msg.int64IO = std::vector<uint64_t>(size[3]);
    msg.boolIO = size[4];

    recv(reinterpret_cast<char *>(&msg.doubleIO[0]), sizeof(double)*size[1]);
    recv(reinterpret_cast<char *>(&msg.doubleIO2[0]), sizeof(double)*size[2]);
    recv(reinterpret_cast<char *>(&msg.int64IO[0]), sizeof(uint64_t)*size[3]);

    // TODO: exception when return of recv is -1
    return msg;
}

PS: Spontane Verbesserungsvorschläge für den Code werden natürlich gerne angenommen.

tntnet

Wenn es denn so spontan sein soll, dann hätte ich was: bei EINTR ist status = -1, also ziehst Du von den bisher empfangenen Bytes 1 ab. Das ist sicher nicht so gemeint. Mach einfach mal ein else vor dem received += status; .

knivil

if (status == -1 && status != EINTR) {

Das ist Quatsch. Hast du mal die Dokumentation zu gelesen? recv() gibt niemals EINTR zurueck. Falls recv() fehl schlaegt wird errno gesetzt. errno ist abzufragen.

mikemodanoxxx

Oh ja, danke. So war eigentlich der Plan :).