Problem mit recv und endlos schleife

[314159265358979]

Ich hatte gerade etwas Zeit und habe deinen Server neu geschrieben. Da ich nicht weiß, wie du deine Daten parst, habe ich erstmal das Parsing weggelassen. Da du direkt auf die C-API's zugreifst, (weiß der Teufel wieso) mache ich das auch so. Der Code besteht aus 4 Dateien. (3 Header + main.cpp) Ich hoffe mal, du erkennst einen Unterschied

socket.hpp:

#ifndef SOCKET_HPP
#define SOCKET_HPP

#include <stdexcept>
#include <string>
#include <cstdint>
#include <cstddef>
#include <vector>

#include <sys/socket.h>
#include <netinet/in.h>

typedef int sockfd_type;

inline void close_socket(sockfd_type sockfd)
{
	if(sockfd != -1)
		close(sockfd);
}

struct socket_error : std::runtime_error
{
	socket_error(std::string const& what)
		: runtime_error(what)
	{}
};

class socket_data
{
	friend class acceptor;
	friend class client_socket;

	sockfd_type sockfd;
};

struct client_socket
{
	client_socket(socket_data data)
		: sockfd(data.sockfd)
	{}

	client_socket(client_socket&& other)
		: sockfd(other.sockfd)
	{
		other.sockfd = -1;
	}

	client_socket& operator = (client_socket&& other)
	{
		close_socket(sockfd);
		sockfd = other.sockfd;
		other.sockfd = -1;
		return *this;
	}

	~client_socket()
	{
		close_socket(sockfd);
	}

	// Schlechte Methode, um zeilenweise zu lesen. Damit ich nicht alles mache, darfst du einen Buffer in die Klasse einbauen.
	std::string read_line()
	{
		std::string result;

		while(peek_two_chars() != "\r\n")
			result += read_char();

		skip_two_chars();

		return result;
	}

	void write(std::string const& str)
	{
		char const* data = str.c_str();
		std::size_t size = str.size();

		int sendret;
		while(size)
		{
			sendret = send(sockfd, data, size, 0);
			data += sendret;
			size -= sendret;
		}

		if(sendret == -1)
			throw socket_error("disconnected");
	}

	client_socket(client_socket const&) = delete;
	client_socket& operator = (client_socket const&) = delete;

private:
	std::string peek_two_chars() { return read_exactly(2, MSG_PEEK); }
	void skip_two_chars() { read_exactly(2, 0); }
	std::string read_char() { return read_exactly(1, 0); }

	std::string read_exactly(std::size_t n, int flags)
	{
		std::vector<char> buf(n);

		std::size_t recvret = recv(sockfd, buf.data(), n, flags);

		switch(recvret)
		{
		case -1:
			throw socket_error("recv() failed");

		case 0:
			throw socket_error("disconnected");

		default:
			if(recvret == n)
				return std::string(buf.begin(), buf.end());

			return std::string(buf.begin(), buf.end()) + read_exactly(n - recvret, flags);
		}
	}

	sockfd_type sockfd;
};

struct acceptor
{
	acceptor(std::uint16_t port, std::size_t backlog)
		: sockfd(socket(PF_INET, SOCK_STREAM, 0))
	{
		if(sockfd == -1)
			throw socket_error("socket() failed");

		sockaddr_in addr;
		addr.sin_family = AF_INET;
		addr.sin_addr.s_addr = INADDR_ANY;
		addr.sin_port = htons(port);

		if(bind(sockfd, reinterpret_cast<sockaddr const*>(&addr), sizeof addr) == -1)
			throw socket_error("bind() failed");

		if(listen(sockfd, static_cast<int>(backlog)) == -1)
			throw socket_error("listen() failed");
	}

	acceptor(acceptor&& other)
		: sockfd(other.sockfd)
	{
		other.sockfd = -1;
	}

	acceptor& operator = (acceptor&& other)
	{
		close_socket(sockfd);
		sockfd = other.sockfd;
		other.sockfd = -1;
		return *this;
	}

	~acceptor()
	{
		close_socket(sockfd);
	}

	client_socket accept()
	{
		socket_data data;

		if((data.sockfd = ::accept(sockfd, 0, 0)) == -1)
			throw socket_error("accept() failed");

		return client_socket(data);
	}

	acceptor(acceptor const&) = delete;
	acceptor& operator = (acceptor const&) = delete;

private:
	sockfd_type sockfd;
};

#endif // SOCKET_HPP

thread.hpp:

#ifndef THREAD_HPP
#define THREAD_HPP

#include <functional>
#include <utility>
#include <pthread.h>

struct thread
{
	thread(std::function<void()> f)
		: functor(std::move(f))
	{
		pthread_create(&handle, 0, &thread::thread_func, this);
	}

	thread(thread&& other)
		: handle(other.handle)
		, functor(std::move(other.functor))
	{
		other.handle = pthread_t();
	}

	thread& operator = (thread&& other)
	{
		pthread_detach(handle);
		handle = other.handle;
		functor = std::move(other.functor);
		other.handle = pthread_t();
		return *this;
	}

	void join()
	{
		pthread_join(handle, 0);
	}

	~thread()
	{
		pthread_detach(handle);
	}

	static void* thread_func(void* this_)
	{
		static_cast<thread*>(this_)->functor();
		return 0;
	}

	thread(thread const&) = delete;
	thread& operator = (thread const&) = delete;

private:
	pthread_t handle;
	std::function<void()> functor;
};

#endif // THREAD_HPP

thread_group.hpp:

#ifndef THREAD_GROUP_HPP
#define THREAD_GROUP_HPP

#include <functional>
#include <utility>
#include <vector>

#include "thread.hpp"

struct thread_group
{
	void create_thread(std::function<void()> f)
	{
		threads.emplace_back(std::move(f));
	}

	~thread_group()
	{
		for(thread& t : threads)
			t.join();
	}

private:
	std::vector<thread> threads;
};

#endif // THREAD_GROUP_HPP

main.cpp:

#include <iostream>
#include <vector>
#include <memory>
#include <cstdlib>

#include "socket.hpp"
#include "thread_group.hpp"

// Der shared_ptr ist ein Workaround für einen Bug in der GCC-Standardlib. Ich würde hier lieber direkt einen client_socket& nehmen.
void handle_client(std::shared_ptr<client_socket>& socket_ptr)
{
	try
	{
		// Die Ausgaben sind hier nicht synchronisiert, das sollte noch gemacht werden.
		for(std::string line; (line = socket_ptr->read_line()) != "";)
			std::cout << line << '\n';

		std::cout << std::endl;

		// Hier ist Platz für deinen Code.
	}

	catch(socket_error const& e)
	{
		std::cerr << e.what() << '\n';
	}
}

int main(int argc, char** argv)
{
	int const default_backlog = 42;

	if(argc < 2 || argc > 3)
	{
		std::cerr << "usage: mysuperhttpserver <port> [backlog=" << default_backlog << ']';
		return 1;
	}

	int const port = std::atoi(argv[1]);
	int const backlog = argc == 3 ? std::atoi(argv[2]) : default_backlog;

	thread_group threads;

	try
	{
		acceptor acc(port, backlog);

		for(;;)
		{
			std::shared_ptr<client_socket> socket = std::make_shared<client_socket>(acc.accept());
			threads.create_thread(std::bind(&handle_client, std::move(socket)));
		}
	}

	catch(socket_error const& e)
	{
		std::cerr << e.what() << '\n';
		return 2;
	}
}

Falls irgendwas unklar sein sollte, einfach fragen. Ich hoffe, du kopierst den Code nicht einfach

[Tuxist]

std::vector<char> buf(n);

Verstehe nicht warum ich Daten von denen ich noch nicht weiß ob sie zusammenhängend sind eine nummerierte liste schreiben soll ? Es werden immer 255 zeichen geholt aber es könnte auch 1/4 oder 1/6 ... whatever von einem Dokument sein. Die Daten können auch unvollständig sein. Ich verstehe nicht wie mir dar eine Liste hilfreich sein kann ? Das Problem ist das http die Verbindung offen hält und deswegen das Parsen Asyncron von httpd laufen muss.

[Ethon]

std::vector<char>(n) ist ähnlich nutzbar wie new char[n], nur deutlich nützlicher vom Umfang und leichter zu verwenden.

Du brauchst einfach einen Eingangsbuffer, der recv emntgegennimmt. Was du dann damit machst ist dir überlassen.

[Tuxist]

jup dieser Eigangsbuffer muss anhand des http Startheader in aufgeteilt werden, hatte hierbei an memmmove gedacht um die Daten weiter zu verarbeiten was man z.b. in einem Vector oder Array machen kann. Die Kompletten http Daten könnte man dann mit strsep in Kopf und Daten aufteilen und dann einen cgiparser laufen lassen.

[DaRe]

Ethon schrieb:

Der Andere: Lies die Dokumentation von recv bevor du "hilfst", ist ja grauenhaft.

Ich vermute, du sprichst mich damit an.
Auf welchen Beitrag beziehst du dich?

[314159265358979]

Tuxist schrieb:

Das Problem ist das http die Verbindung offen hält und deswegen das Parsen Asyncron von httpd laufen muss.

Ist doch gar kein Problem. Für jeden Client wird ein eigener Thread erstellt, in dem dieser dann bearbeitet wird. Eine professionellere Lösung hätte vermutlich Worker-Threads verwendet, da hier pro Client ein Thread quasi-geleakt wird. (Die Threads werden erst bei Programmende aufgeräumt.)

Tuxist schrieb:

jup dieser Eigangsbuffer muss anhand des http Startheader in aufgeteilt werden, hatte hierbei an memmmove gedacht um die Daten weiter zu verarbeiten was man z.b. in einem Vector oder Array machen kann. Die Kompletten http Daten könnte man dann mit strsep in Kopf und Daten aufteilen und dann einen cgiparser laufen lassen.

Pfui pfui pfui!
Du kriegst die Daten vom Socket doch schon zeilenweise als std::string's. Du brauchst also nur noch diese Zeilen zu verarbeiten und dem Client eine Antwort zu senden.

[Ethon]

DaRe schrieb:

Ethon schrieb:

Der Andere: Lies die Dokumentation von recv bevor du "hilfst", ist ja grauenhaft.

Ich vermute, du sprichst mich damit an.
Auf welchen Beitrag beziehst du dich?

Ich meine sdfsdfsdf.

[Tuxist]

314159265358979:

Die Zeilen müssen aber nicht komplett sein dar recv immer nur 255 zeichen in den Buffer schreibt und ein habler Post Datensatz zum beispiel lässt sich schlecht parsen. Deswegen muss alles zuerst in einen Buffer und dann verarbeitet werden.
Die Zeilen beinhalten nicht unbedingt einen kompletten Datensatz das kann ich erst im Buffer prüfen. Oder habe ich jetzt recv() falsch verstanden ? Der Rückgabe wert sagt Lediglich über die anzahl der Zeichen im Bitstream was aus ? Und der char* muss eine Festdefinierte länge haben ?

[314159265358979]

recv liest so viel, wie du ihm sagst.

[theta]

314159265358979 schrieb:

recv liest so viel, wie du ihm sagst.

Nö. recv(..) liesst maximal soviel wie du ihm sagst, manchmal durchaus auch weniger.
Es braucht ein Frameing Protokoll, welches sagt wo eine Meldung (hier ein String) aufhört und wo der nächste anfängt.

[314159265358979]

theta schrieb:

Nö. recv(..) liesst maximal soviel wie du ihm sagst, manchmal durchaus auch weniger.

Das meinte ich ja. Steht doch eh implizit da.

[theta]

314159265358979 schrieb:

theta schrieb:

Nö. recv(..) liesst maximal soviel wie du ihm sagst, manchmal durchaus auch weniger.

Das meinte ich ja. Steht doch eh implizit da.

Genau.

[314159265358979]

Wenn ich's nicht wüsste, würde ich recv wohl kaum wiederholt aufrufen.

[Tuxist]

Nur mal eben zum zusammen fassen folgendes muss beachtet werden

-recv liest einen bitstream in einer vorgebenden maximalen länge ein die aber durch aus unterschritten werden kann.

-Die Connection wird nicht vom einem Client nach einer Anfrage nicht geschlossen, das heißt es muss ein thread im Hintgrund laufen und einer die Daten verarbeiten.

-Der Buffer für die Daten von recv muss dynamisch in der Größe sein

-Pro Anfrage einen Thread macht am meisten sinn dar man so optimal skalieren kann.

Also ich komme auf eine dynamische anzahl von threads in denen jeweils 2 threads laufen ?

[theta]

Tuxist schrieb:

-recv liest einen bitstream in einer vorgebenden maximalen länge ein die aber durch aus unterschritten werden kann.

Also ein byte-Stream ist es schon...

Tuxist schrieb:

-Die Connection wird nicht vom einem Client nach einer Anfrage nicht geschlossen, das heißt es muss ein thread im Hintgrund laufen und einer die Daten verarbeiten.

Da ist vermutlich ein nicht zuviel - obs einen zweiten Thread braucht ist eine andere Frage.

Tuxist schrieb:

-Der Buffer für die Daten von recv muss dynamisch in der Größe sein

Ja, wenn das Framing Protokoll Frames von unterschiedlicher Länge vorsieht, falls nicht, dann nicht. Also: It depends.

Tuxist schrieb:

-Pro Anfrage einen Thread macht am meisten sinn dar man so optimal skalieren kann.

Eigentlich skaliert das sehr schlecht, geht aber gut für den Anfang und für wenige Verbindungen.

[Tuxist]

Framing Protokoll Frames

Also dieser begriff ist mir nicht geläufig du meinst sicherlich die größe der Frame Packete ? Welche bei http variieren können.

Eigentlich skaliert das sehr schlecht, geht aber gut für den Anfang und für wenige Verbindungen.

Stattdessen Threads für die einzelnen Verarbeitungsschritte ?

[hjkhjkhjk]

Wenn du etwas gut skalierendes brauchst, dann kommst du um die API des Betriebssystems nicht durm herum. Für den Anfang könntest du dir aber vielleicht mit Threads + select was zusammenbasteln.

[cooky451]

sdfsdfsdf schrieb:

Es gibt diese Stelle sogesehen nicht, da im Standard nicht geschrieben wird, dass der Speicher zusammenhängend sein muss. Ganz anders beim std::vector, wo dies explizit verlangt und somit garantiert ist.

Das ist nicht (mehr) korrekt. Siehe auch

n3242 21.4.1 basic_string general requirements 5 schrieb:

The char-like objects in a basic_string object shall be stored contiguously. That is, for any basic_string
object s, the identity &*(s.begin() + n) == &*s.begin() + n shall hold for all values of n such that 0
<= n < s.size().

[sdfsdfsdf]

cooky451 schrieb:

sdfsdfsdf schrieb:

Es gibt diese Stelle sogesehen nicht, da im Standard nicht geschrieben wird, dass der Speicher zusammenhängend sein muss. Ganz anders beim std::vector, wo dies explizit verlangt und somit garantiert ist.

Das ist nicht (mehr) korrekt. Siehe auch

n3242 21.4.1 basic_string general requirements 5 schrieb:

The char-like objects in a basic_string object shall be stored contiguously. That is, for any basic_string
object s, the identity &*(s.begin() + n) == &*s.begin() + n shall hold for all values of n such that 0
<= n < s.size().

Es ist richtig, dass dies mit dem neuen Standard garantiert wird. Jedoch ist der neue Standard ja noch nicht in Form konformer Compiler in Erscheinung getreten und das wird wohl auch noch dauern. Letztendlich ist dieser Umstand aber eh nicht wirklich relevant, da zumindest mir kein Compiler bekannt ist der es anders handhabt.

[cooky451]

Natürlich ist das relevant. Vorher hatten es nur die Compiler implementiert, jetzt garantiert es auch noch der Standard. Wenn das (der Standard) nicht wichtig wäre, hättest du es ja auch gar nicht erst erwähnen müssen. Schließlich macht es schon einen Unterschied, ob man das ganze Zeug aus einem Vektor in den String kopiert oder eben gleich in den String einliest.