LINUX.ORG.RU

Порт не освобождается после закрытия приложения

 ,


0

2

Здравствуйте. Я тут немного копаюсь в цпп сокетах. Написал вот такой код. После того как я посылаю прерывание Ctrl^C, он пишет мне что функция закрывания вызвалась и все ок. Но если я после этого пытаюсь запустить приложение снова, то в течении где-то полминуты порт недоступен. Я как-то неправильно закрываю сокет?

#include <iostream>
#include <string>
#include <cstring>
#include <unistd.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <signal.h>
#include <thread>
#include <mutex>
#include <set>
#include <chrono>

using namespace std;

class server_exception : public exception {
public:
	string message;
	server_exception(string m) : message(m) {}
};


#define BUF_LEN 1024
#define WITH_MUTEX(mutex, action) {mutex.lock(); action; mutex.unlock();};

bool global_stop_signal = false;

class server {

private:

	bool opened = false;
	set<thread::id> client_threads;
	mutex client_mutex;
	int listener = -1;

	void client(int socket) {
		WITH_MUTEX(client_mutex, client_threads.insert(this_thread::get_id()));
		char buf[BUF_LEN];
		int bytes_readed;
		string message;
		do {
			bytes_readed = recv(socket, (void*)buf, BUF_LEN, 0);
			int signal_on = -1;
			for(int i=0; i<bytes_readed; ++i) {
				if(buf[i] == '\n') {
					signal_on = i;
					break;
				}
			}
			if(signal_on == -1)
				message += buf;
			else {
				if(signal_on != 0 && buf[signal_on - 1] == 13)
					signal_on--;
				for(int i=0; i<signal_on; ++i)
					message += buf[i];
				bytes_readed = 0;
			}
		} while (bytes_readed > 0);
		cout << "message received: \"" << message << "\"" << endl;
		sprintf(buf, "received %d len message\n", static_cast<int>(message.size()));
		send(socket, buf, strlen(buf), 0);
		close(socket);
		WITH_MUTEX(client_mutex, client_threads.erase(this_thread::get_id()));
	}

public:

	void run(unsigned short port) {
		opened = true;
		listener = socket(AF_INET, SOCK_STREAM, 0);
		if(listener < 0) {
			throw server_exception("listener not open");
		}
		sockaddr_in addr;
		bzero((char*)&addr, sizeof(addr));
		addr.sin_family = AF_INET;
		addr.sin_addr.s_addr = INADDR_ANY;
		addr.sin_port = htons(port);
		cout << "connect on " << INADDR_ANY << " " << port << endl;
		if(bind(listener, (sockaddr*) &addr, sizeof(addr)) < 0) {
			throw server_exception("bind error");
		}
		// now on port
		listen(listener, 10);
		for(;;) {
			if(global_stop_signal) {
				return;
			}
			// use accept4(listener, NULL, NULL, SOCK_NONBLOCK)
			int socket = accept(listener, NULL, NULL);
			if(socket < 0) {
				throw server_exception("accept error");
			} else {	
				thread client_thread(&server::client, this, socket);
				client_thread.detach();
			}
		}
	}
	~server() {
		if(opened)
			stop();
	}
	void stop() {
		if(!opened)
			return;
		opened = false;
		cerr << "close call" << endl;
		while(client_threads.size() > 0)
			this_thread::sleep_for(chrono::milliseconds(100));
		if(listener >= 0)
			close(listener);
	}
};

void ctrl_c_event(int _) {
	global_stop_signal = true;
}

int main() {

	struct sigaction handler;

	handler.sa_handler = ctrl_c_event;
	sigemptyset(&handler.sa_mask);
	handler.sa_flags = 0;

	sigaction(SIGINT, &handler, NULL);

	server serv;
	try {
		serv.run(8080);
	} catch (server_exception exc) {
		cout << "exception: " << exc.message << endl;
	}
	serv.stop();

	return 0;
}

★★★★★

Кроме того. Кто-нибудь знает, можно ли тормознуть треды обслуживающие клиентов, после того как для них был вызван detach? Или способа одобренного стандартом нет?

Aswed ★★★★★
() автор топика
Ответ на: комментарий от DELIRIUM

Так, добавил shutdown(socket, 2) перед каждым close. В метод stop тоже. Все равно освобождается не сразу.

Aswed ★★★★★
() автор топика

Код, конечно, вызывает оторопь. Можно хотя бы часть вещей обозначить с альтернативами?

eao197 ★★★★★
()
Ответ на: комментарий от Aswed

SO_REUSEADDR более стандартный, а эффект по идее тот же.

Это вроде из-за TIME-WAIT состояния сокета после его закрытия, т.е. с кодом всё нормально, оно так и должно быть.

xaizek ★★★★★
()
Ответ на: комментарий от Aswed

Насколько я помню, после закрытия сокета порт остается «занятым» еще некоторое время (вроде как до 2мин по умолчанию). Сделано это специально для того, чтобы сетевой интерфейс мог «подобрать» пакеты, которые пришли с опозданием.

Если не выставлять опцию SO_REUSEPORT, то система не даст переиспользовать порт до тех пор, пока не истечет этот тайм-аут. Что не хорошо для ситуаций, когда сервер падает и быстро рестартует. В этом случае SO_REUSEPORT дает возможность быстро рестартовать не дожидаясь истечения тайм-аута.

ЗЫ. Разбирался с этим делом очень давно, мог и соврать.

eao197 ★★★★★
()
Ответ на: комментарий от Aswed

Что именно не нравится?

Очень многое :)

Из того, что сразу напрягло:

class server_exception : public exception {
public:
	string message;
	server_exception(string m) : message(m) {}
};
Тут нужен, как минимум, move при инициализации server_exception::message. Т.е. что-то вроде:
class server_exception : public exception {
public:
	string message;
	server_exception(string m) : message(move(m)) {}
};
Еще лучше было бы отнаследоваться, например, от std::runtime_error:
class server_exception : public runtime_error {
public:
  using runtime_error::runtime_error; // Наследуем конструктор.
};

Далее, макросы зло.

#define BUF_LEN 1024
#define WITH_MUTEX(mutex, action) {mutex.lock(); action; mutex.unlock();};
Без BUF_LEN вообще можно обойтись (покажу ниже). Вместо WITH_MUTEX лучше сделать вспомогательную шаблонную функцию, которая, к тому же, будет освобождать mutex в случае исключения:
template<typename L> void with_mutex(mutex & m, L && l) {
  lock_guard<mutex> lock{m};
  l();
}
Что позволит писать так:
void client(int socket) {
	with_mutex(client_mutex, 
 [&]{ client_threads.insert(this_thread::get_id()); });

По поводу BUF_LEN. В С++ не принято задавать размерности через define. Тем более в C++11 можно обойтись без выделения отдельной константы в этом случае:

void client(int socket) {
	with_mutex(client_mutex, 
 [&]{ client_threads.insert(this_thread::get_id()); });
	array<char, 1024> buf;
	int bytes_readed;
	string message;
	do {
		bytes_readed = recv(socket, buf.data(), buf.size(), 0);
...
Мне еще не нравится то, что тип у bytes_readed (кстати, вроде как по правилам английского должно быть bytes_read) объявлен как int, тогда как recv возвращает ssize_t. Не есть хорошо смешивать целые разных размеров. Поэтому я бы лично делал что-то вроде:
for(;;) {
  const auto bytes_read = recv(socket, ...);
  ...
}
и организовывал бы выходы из цикла через break.

...продолжение следует.

eao197 ★★★★★
()

Какая адская смесь крестов с сями. Какая же религия не позволяет автору использовать ASIO?

asaw ★★★★★
()
Ответ на: комментарий от Aswed

Продолжаем.

Вот тут не уверен в корректности кода:

if(signal_on == -1)
	message += buf;
Вроде как никто не гарантирует, что в конце buf будет 0-символ. Поэтому, если buf будет заполнен ненулевыми значениями, то к message будет добавлен и весь мусор в памяти после завершения buf, до первого найденного нуля или до segmentation fault-а. Безопаснее было бы написать что-то вроде message.append(buf, bytes_read) или message.append(buf.data(), buf.data() + bytes_read);.

Вот это напоминает детский сад:

for(int i=0; i<signal_on; ++i)
	message += buf[i];
Если нужно добавить в message первые signal_on символов из buf, то это делается через message.apped(buf, signal_on).

Ну и вот здесь:

try {
	serv.run(8080);
} catch (server_exception exc) {
	cout << "exception: " << exc.message << endl;
}
исключение нужно ловить по константной ссылке, а не по значению:
try {
	serv.run(8080);
} catch (const server_exception & exc) {
	cout << "exception: " << exc.message << endl;
}

Ну это так, первое что в глаза бросилось. Еще с разными поисками в реализации метода client можно было бы воспользоваться стандартными алгоритмами. Но это уже совсем полная переделка будет :)

eao197 ★★★★★
()
		for(;;) {
			if(global_stop_signal) {
				return;
			}
			// use accept4(listener, NULL, NULL, SOCK_NONBLOCK)
			int socket = accept(listener, NULL, NULL);
			if(socket < 0) {
				throw server_exception("accept error");
			} else {	
				thread client_thread(&server::client, this, socket);
				client_thread.detach();
			}
		}

И чего, этот цикл без Ctrl+C прерываться не будет? Зачем делать client_thread.detach(), если ты всё равно следишь за всеми потоками? Только чтоб пропустить баг и сделать утечку ресурсов...

void ctrl_c_event(int _) {
	global_stop_signal = true;
}

Здесь вообще атомики должны быть по-хорошему... В общем, ASIO у этого кода по-любому выигрывает.

asaw ★★★★★
()
Последнее исправление: asaw (всего исправлений: 1)

Используй RAII и твоя проблема решится сама собой.

invy ★★★★★
()

eao197, asaw Спасибо за советы. Никак не могу избавиться от привычки писать как на С.

Aswed ★★★★★
() автор топика
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.