LINUX.ORG.RU

Сокет и race condition


0

1

Ребят, покритикуйте код. Посоветуйте, что можно сделать? На десятках соединений код работает. На тысяче все встает раком.

Суть в том, что первая запущенная программа должна успеть создать сокет, а остальные копии программы к нему уже подключаются. Но, видимо, копии запускаются быстрее, чем создается сокет и начинается гонка.

	const gchar *tdir = g_get_tmp_dir ();			//tmp dir
	const gchar *uname = g_get_user_name ();		//user name

	sock_name = g_strconcat (tdir,"/japw-",uname,NULL);

	s_id = socket (AF_UNIX,SOCK_STREAM,0);

	gint s_status;
	struct sockaddr_un addr;
	g_stpcpy (addr.sun_path, sock_name);
	addr.sun_family = AF_UNIX;

	s_status = bind (s_id,(struct sockaddr *) &addr, sizeof(addr));
	gint s_c_status;
	if (s_status == -1) {
		gint i = 0;
		do {
			s_c_status = connect (s_id,(struct sockaddr *) &addr, sizeof(addr));
			++i;
		} while (s_c_status == -1 && i < 1500);
		if (s_c_status != -1) {
			create_listf (fcount, flist);
			if (max_count_image == 0) exit (EXIT_SUCCESS); 
			gchar *buf;
			buf = g_strjoinv ("|", fnames);
			channel = g_io_channel_unix_new (s_id);

			gsize bw,lw;
			lw = strlen (buf) +1;
			g_io_channel_write (channel, buf, lw, &bw);
			g_io_channel_shutdown (channel, TRUE, NULL);
			close (s_id);
			g_free (buf);
			exit (EXIT_SUCCESS);
		}
		else {
			(void) unlink (sock_name);
			s_status = bind (s_id,(struct sockaddr *) &addr, sizeof(addr));
			if (s_status == -1) {
				g_printf ("Error: create socket");
				exit (EXIT_FAILURE);
			}
		}
	}
	listen (s_id, 5);
	channel = g_io_channel_unix_new (s_id);

	s_watch = g_io_add_watch (channel, G_IO_IN, (GIOFunc) accept_files, NULL);

★★★★★

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

Ответ на: комментарий от ananas

Запуском из файл-менеджера. При множественном выделении, файл-менеджер запускает отдельную копию на каждый файл.

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

а первый запущенный экземпляр гарантированно дожидается завершения работы всех остальных? подозреваю, что нет и SO_REUSEADDR должен помочь

ananas ★★★★★
()

и еще, perror() иногда очень сильно помогает в понимании проблемы.

ananas ★★★★★
()

Критика немного не в тему.

listen(),unlink() возвращают значения, а значит их НАДО проверять. Если возникновение ошибки при нормальной работе не ожидается, то ставь заглушки. У меня обычно макрос вида try_call( listen(s_id,5) ); В этом макросе проверяется возврат на listen(s_id,5)==-1 и в случае ошибки программа отвалится выбросив на консоль сообщение. Такой стиль как у тебя ИМХО должен приносить много страданий себе и окружающим программистам. Если ты не любитель BDSM лучше для каждого вызова делай явную проверку или ставь заглушку.

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

close() по идее может вернуть EINTR. Тогда я так понял надо его запускать по новой. От греха подальше лучше тоже сделать обертку над вызовом close(), которая будет заново вызывать close() в случае возврата EINTR и отваливаться с сообщением в случае другой ошибки.

pathfinder ★★★★
()

Теперь ближе к делу.

Пускай у нас три процесса (А,B,C).

A: bind(), сокет создан

B: bind()==-1 переходим к connect()

C: bind()==-1 переходим к connect()

A: завершается, сокет мертвый

B: не может установить connect(),unlink(),bind(),сокет создан

C: не может установить connect(),unlink(),bind()==-1,WTF?!!

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

Может что-то в таком духе:

bool isServer=true;

while(true)
{
    {
        //Пускай это будет класс который в конструкторе вызывает flock(self_program_fd,LOCK_EX)
        //а в деструкторе flock(self_program_fd,LOCK_UN)
        FlockWatcher watch(self_program_fd);

        int r_bind = bind(...);
        if(r_bind==-1)
        {
            if(errno==EADDRINUSE) //socket file exist?
            {
                int r_connect = connect(...);
                if(r_connect==-1)
                {
                    if(errno==ECONNREFUSED)
                    {
                        try_call( unlink(...) );
                    }
                    continue;
                }
                else
                {
                    isServer=false;
                    break;
                }
            }
            else
            {
                //panic_panic_PANIC();
                continue;
            }
        }
        else
        {
            break;
        }
    }
}

if(isServer)
{
    ProcessAsServer();
}
else
{
    ProcessAsClient();
}

Что народ думает над таким подходом?

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

Только надо завернуть код в процедуры для того, чтобы скобок и if-ов было меньше.

И еще проверить смысл кодов EADDRINUSE и ECONNREFUSED. А то я не уверен, что все правильно понял.

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