On Fri, Sep 11, 2009 at 10:59:41PM +0200, Roland Clobus wrote: > On Wed, 2009-09-09 at 10:11 -0700, Steve Langasek wrote: > > Well, that's not a proper fix for the bug then. The server should be > > listening on both IPv6 and IPv4. > > I think the bug that was reported: 'Unable to play games using > localhost' is adequately fixed by adapting the clients. > I agree that it should be nice if the server would listen on all > available protocols. IMO that would be another bug report with severity > 'wishlist', while this bug with severity 'grave' can be closed.
I've written a patch which listens on everything returned by getaddrinfo. The call returns two structures, but the second bind fails with "address already in use". It seems the problem doesn't occur in Debian, because it automatically listens on both IPv4 and IPv6(?) So I'm a bit confused: this is a Debian bug, so the problem presumably exists on Debian systems. This may be different on GNU/kFreeBSD, but the reporter is using GNU/Linux, like the system I'm testing on. Anyway, I'm attaching the patch. Thanks, Bas
Index: common/network.h
===================================================================
--- common/network.h (revision 1459)
+++ common/network.h (working copy)
@@ -82,9 +82,10 @@
* @param port The port
* @retval error_message If opening fails, a description of the error
* You should g_free the error_message
- * @return A file descriptor if succesfull, or -1 if it fails
+ * @retval num_fds The number of valid fds in the returned array
+ * @return An array of file descriptors if succesfull, or NULL if it fails
*/
-int net_open_listening_socket(const gchar * port, gchar ** error_message);
+int *net_open_listening_socket(const gchar * port, gchar ** error_message, int *num_fds);
/** Close a socket
*/
Index: common/network.c
===================================================================
--- common/network.c (revision 1459)
+++ common/network.c (working copy)
@@ -743,14 +743,17 @@
return pioneers_dir;
}
-int net_open_listening_socket(const gchar * port, gchar ** error_message)
+int *net_open_listening_socket(const gchar * port, gchar ** error_message, int *num_fds)
{
#ifdef HAVE_GETADDRINFO_ET_AL
int err;
struct addrinfo hints, *ai, *aip;
int yes;
- gint fd = -1;
+ int *fds;
+ int max_num_fds;
+ *num_fds = 0;
+
memset(&hints, 0, sizeof(hints));
hints.ai_family = NETWORK_PROTOCOL;
@@ -763,65 +766,77 @@
g_strdup_printf(_(""
"Error creating struct addrinfo: %s"),
gai_strerror(err));
- return -1;
+ return NULL;
}
+ max_num_fds = 0;
for (aip = ai; aip; aip = aip->ai_next) {
- fd = socket(aip->ai_family, SOCK_STREAM, 0);
- if (fd < 0) {
+ ++max_num_fds;
+ }
+
+ fds = g_malloc (max_num_fds * sizeof (*fds));
+ for (aip = ai; aip; aip = aip->ai_next) {
+ fds[*num_fds] = socket(aip->ai_family, SOCK_STREAM, 0);
+ if (fds[*num_fds] < 0) {
continue;
}
- yes = 1;
/* setsockopt() before bind(); otherwise it has no effect! -- egnor */
+ yes = 1;
if (setsockopt
- (fd, SOL_SOCKET, SO_REUSEADDR, &yes,
+ (fds[*num_fds], SOL_SOCKET, SO_REUSEADDR, &yes,
sizeof(yes)) < 0) {
- net_closesocket(fd);
+ g_warning(_(""
+ "Not using socket because setting REUSEADDR failed: %s\n"),
+ net_errormsg());
+ net_closesocket(fds[*num_fds]);
continue;
}
- if (bind(fd, aip->ai_addr, aip->ai_addrlen) < 0) {
- net_closesocket(fd);
+ if (bind(fds[*num_fds], aip->ai_addr, aip->ai_addrlen) < 0) {
+ g_warning(_(""
+ "Not using socket because bind failed: %s\n"),
+ net_errormsg());
+ net_closesocket(fds[*num_fds]);
continue;
}
- break;
- }
+ if (net_set_socket_non_blocking(fds[*num_fds])) {
+ g_warning(_(""
+ "Not using socket because it cannot be set to non-blocking: %s\n"),
+ net_errormsg());
+ net_closesocket(fds[*num_fds]);
+ continue;
+ }
- if (!aip) {
- *error_message =
- g_strdup_printf(_(""
- "Error creating listening socket: %s\n"),
- net_errormsg());
- freeaddrinfo(ai);
- return -1;
+ if (listen(fds[*num_fds], 5) < 0) {
+ g_warning(_(""
+ "Not using socket because listening failed: %s\n"),
+ net_errormsg());
+ net_closesocket(fds[*num_fds]);
+ continue;
+ }
+
+ /* Listening succeeded. Keep the fd and continue. */
+ ++*num_fds;
}
freeaddrinfo(ai);
- if (net_set_socket_non_blocking(fd)) {
+ if (*num_fds == 0) {
+ g_free (fds);
*error_message =
g_strdup_printf(_(""
- "Error setting socket non-blocking: %s\n"),
+ "Error creating listening socket: %s\n"),
net_errormsg());
- net_closesocket(fd);
- return -1;
+ return NULL;
}
- if (listen(fd, 5) < 0) {
- *error_message =
- g_strdup_printf(_(""
- "Error during listen on socket: %s\n"),
- net_errormsg());
- net_closesocket(fd);
- return -1;
- }
*error_message = NULL;
- return fd;
+ return fds;
#else /* HAVE_GETADDRINFO_ET_AL */
*error_message =
g_strdup(_("Listening not yet supported on this platform."));
- return -1;
+ return NULL;
#endif /* HAVE_GETADDRINFO_ET_AL */
}
Index: meta-server/main.c
===================================================================
--- meta-server/main.c (revision 1459)
+++ meta-server/main.c (working copy)
@@ -92,7 +92,8 @@
static fd_set read_fds;
static fd_set write_fds;
-static int accept_fd;
+static int *accept_fds;
+static int num_accept_fds;
static gint max_fd;
/* Command line data */
@@ -188,8 +189,14 @@
static void find_new_max_fd(void)
{
GList *list;
+ int i;
- max_fd = accept_fd;
+ max_fd = -1;
+ for (i = 0; i < num_accept_fds; ++i) {
+ if (accept_fds[i] > max_fd) {
+ max_fd = accept_fds[i];
+ }
+ }
for (list = client_list; list != NULL; list = g_list_next(list)) {
Client *client = list->data;
@@ -835,7 +842,7 @@
set_client_event_at(client);
}
-static void accept_new_client(void)
+static void accept_new_client(int accept_fd)
{
int fd;
gchar *error_message;
@@ -927,6 +934,7 @@
fd_set read_res;
fd_set write_res;
int num;
+ int i;
struct timeval *timeout;
GList *list;
@@ -951,9 +959,11 @@
}
}
- if (FD_ISSET(accept_fd, &read_res)) {
- accept_new_client();
- num--;
+ for (i = 0; i < num_accept_fds; ++i) {
+ if (FD_ISSET(accept_fds[i], &read_res)) {
+ accept_new_client(accept_fds[i]);
+ num--;
+ }
}
list = client_list;
@@ -984,17 +994,20 @@
static gboolean setup_accept_sock(const gchar * port)
{
- int fd;
+ int *fds;
+ int i;
gchar *error_message;
- fd = net_open_listening_socket(port, &error_message);
- if (fd == -1) {
+ fds = net_open_listening_socket(port, &error_message, &num_accept_fds);
+ if (!fds) {
my_syslog(LOG_ERR, "%s", error_message);
g_free(error_message);
return FALSE;
}
- accept_fd = max_fd = fd;
- FD_SET(accept_fd, &read_fds);
+ accept_fds = fds;
+ find_new_max_fd ();
+ for (i = 0; i < num_accept_fds; ++i)
+ FD_SET(accept_fds[i], &read_fds);
return TRUE;
}
@@ -1063,6 +1076,7 @@
GOptionContext *context;
GError *error = NULL;
GList *game_list;
+ int i;
setlocale(LC_ALL, "");
bindtextdomain(PACKAGE, LOCALEDIR);
@@ -1135,6 +1149,11 @@
my_syslog(LOG_INFO, "Pioneers meta server started.");
select_loop();
+ for (i = 0; i < num_accept_fds; ++i) {
+ net_closesocket(accept_fds[i]);
+ accept_fds[i] = -1;
+ }
net_finish();
+ g_free (accept_fds);
return 0;
}
Index: server/server.c
===================================================================
--- server/server.c (revision 1459)
+++ server/server.c (working copy)
@@ -74,8 +74,9 @@
game = g_malloc0(sizeof(*game));
- game->accept_tag = 0;
- game->accept_fd = -1;
+ game->accept_tag = NULL;
+ game->accept_fd = NULL;
+ game->num_fds = 0;
game->is_running = FALSE;
game->is_game_over = FALSE;
game->params = params_copy(params);
@@ -101,6 +102,10 @@
if (game->server_port != NULL)
g_free(game->server_port);
params_free(game->params);
+ if (game->accept_fd)
+ g_free (game->accept_fd);
+ if (game->accept_tag)
+ g_free (game->accept_tag);
g_free(game);
}
@@ -161,14 +166,14 @@
}
-static void player_connect(Game * game)
+static void player_connect(Connect_info * info)
{
gchar *location;
- gint fd = accept_connection(game->accept_fd, &location);
+ gint fd = accept_connection(info->game->accept_fd[info->num], &location);
if (fd > 0) {
- if (player_new_connection(game, fd, location) != NULL)
- stop_timeout(game);
+ if (player_new_connection(info->game, fd, location) != NULL)
+ stop_timeout(info->game);
}
g_free(location);
}
@@ -177,21 +182,28 @@
const gchar * meta_server_name)
{
gchar *error_message;
+ int i;
game->accept_fd =
- net_open_listening_socket(game->server_port, &error_message);
- if (game->accept_fd == -1) {
+ net_open_listening_socket(game->server_port, &error_message, &game->num_fds);
+ if (game->accept_fd == NULL) {
log_message(MSG_ERROR, "%s\n", error_message);
g_free(error_message);
return FALSE;
}
+ game->accept_tag = g_malloc0 (game->num_fds * sizeof (*game->accept_tag));
game->is_running = TRUE;
start_timeout(game);
- game->accept_tag = driver->input_add_read(game->accept_fd,
- (InputFunc)
- player_connect, game);
+ game->connect_info = g_malloc0(game->num_fds * sizeof (Connect_info));
+ for (i = 0; i < game->num_fds; ++i) {
+ game->connect_info[i].game = game;
+ game->connect_info[i].num = i;
+ game->accept_tag[i] = driver->input_add_read(game->accept_fd[i],
+ (InputFunc)
+ player_connect, &game->connect_info[i]);
+ }
if (register_server) {
g_assert(meta_server_name != NULL);
@@ -254,6 +266,7 @@
gboolean server_stop(Game * game)
{
GList *current;
+ int i;
if (!server_is_running(game))
return FALSE;
@@ -262,13 +275,18 @@
game->is_running = FALSE;
if (game->accept_tag) {
- driver->input_remove(game->accept_tag);
- game->accept_tag = 0;
+ for (i = 0; i < game->num_fds; ++i)
+ driver->input_remove(game->accept_tag[i]);
+ g_free (game->accept_tag);
+ game->accept_tag = NULL;
}
- if (game->accept_fd >= 0) {
- close(game->accept_fd);
- game->accept_fd = -1;
+ if (game->accept_fd) {
+ for (i = 0; i < game->num_fds; ++i)
+ close(game->accept_fd[i]);
+ g_free (game->accept_fd);
+ game->accept_fd = NULL;
}
+ game->num_fds = 0;
playerlist_inc_use_count(game);
current = game->player_list;
Index: server/server.h
===================================================================
--- server/server.h (revision 1459)
+++ server/server.h (working copy)
@@ -47,7 +47,13 @@
#define TERRAIN_RANDOM 1
typedef struct Game Game;
+
typedef struct {
+ Game *game; /* The game */
+ int num; /* The index into game->accept_fd */
+} Connect_info;
+
+typedef struct {
StateMachine *sm; /* state machine for this player */
Game *game; /* game that player belongs to */
@@ -89,8 +95,10 @@
GameParams *params; /* game parameters */
gchar *hostname; /* reported hostname */
- int accept_fd; /* socket for accepting new clients */
- int accept_tag; /* Gdk event tag for accept socket */
+ int num_fds; /* number of elements in accept_fd and accept_tag */
+ int *accept_fd; /* sockets for accepting new clients */
+ int *accept_tag; /* Gdk event tags for accept socket */
+ Connect_info *connect_info; /* Structure for finding the listening fd in the accept callback */
GList *player_list; /* all players in the game */
GList *dead_players; /* all players that should be removed when player_list_use_count == 0 */
Index: server/admin.c
===================================================================
--- server/admin.c (revision 1459)
+++ server/admin.c (working copy)
@@ -35,7 +35,8 @@
#include "server.h"
/* network administration functions */
-comm_info *_accept_info = NULL;
+static comm_info *_accept_info = NULL;
+static int num_accept_fds = 0;
gint admin_dice_roll = 0;
@@ -373,25 +374,35 @@
void admin_listen(const gchar * port)
{
gchar *error_message;
+ int num_fds;
+ int *fds;
+ int i;
- if (!_accept_info) {
- _accept_info = g_malloc0(sizeof(comm_info));
- }
-
- /* open up a socket on which to listen for connections */
- _accept_info->fd = net_open_listening_socket(port, &error_message);
- if (_accept_info->fd == -1) {
+ /* open up sockets on which to listen for connections */
+ fds = net_open_listening_socket(port, &error_message, &num_fds);
+ if (!fds) {
log_message(MSG_ERROR, "%s\n", error_message);
g_free(error_message);
return;
}
+
+ if (!_accept_info) {
+ _accept_info = g_malloc0(sizeof(comm_info) * num_fds);
+ } else if (num_accept_fds != num_fds) {
+ g_free (_accept_info);
+ _accept_info = g_malloc0(sizeof(comm_info) * num_fds);
+ }
+ num_accept_fds = num_fds;
+
+ for (i = 0; i < num_fds; ++i) {
#ifdef PRINT_INFO
- g_print("admin_listen: fd = %d\n", _accept_info->fd);
+ g_print("admin_listen: fd[%d] = %d\n", i, fds[i]);
#endif
-
- /* set up the callback to handle connections */
- _accept_info->read_tag =
- driver->input_add_read(_accept_info->fd,
- (InputFunc) admin_connect,
- _accept_info);
+ _accept_info[i].fd = fds[i];
+ /* set up the callback to handle connections */
+ _accept_info[i].read_tag =
+ driver->input_add_read(fds[i],
+ (InputFunc) admin_connect,
+ &_accept_info[i]);
+ }
}
Index: server/admin.h
===================================================================
--- server/admin.h (revision 1459)
+++ server/admin.h (working copy)
@@ -25,7 +25,7 @@
#include "network.h"
typedef struct _comm_info {
- gint fd;
+ int fd;
guint read_tag;
guint write_tag;
} comm_info;
Index: Makefile.am
===================================================================
--- Makefile.am (revision 1459)
+++ Makefile.am (working copy)
@@ -61,7 +61,7 @@
desktopdir = $(datadir)/applications
# Let object files be generated in their own subdirectories
-AUTOMAKE_OPTIONS = subdir-objects
+AUTOMAKE_OPTIONS = subdir-objects foreign
# set up these variables so the included Makefile.ams can use +=
SUBDIRS =
signature.asc
Description: Digital signature
