From 38a608c55af7654f23c9a16129ab6211aac3b7ab Mon Sep 17 00:00:00 2001 From: Martin Mares Date: Mon, 31 May 2004 21:48:19 +0000 Subject: [PATCH] Rewritten the I/O loop. All socket operations are now safe, meaning that you can delete the socket from anywhere in the hooks and nothing should break. Also, the receive/transmit buffers are now regular xmalloc()'ed buffers, not separate resources which would need shuffling around between pools. sk_close() is gone, use rfree() instead. --- lib/socket.h | 8 +-- proto/bgp/bgp.c | 4 +- sysdep/unix/io.c | 141 ++++++++++++++++++++++++++------------------- sysdep/unix/main.c | 2 +- 4 files changed, 89 insertions(+), 66 deletions(-) diff --git a/lib/socket.h b/lib/socket.h index 147e5ce1..bc4525f7 100644 --- a/lib/socket.h +++ b/lib/socket.h @@ -1,7 +1,7 @@ /* * BIRD Socket Interface * - * (c) 1998--1999 Martin Mares + * (c) 1998--2004 Martin Mares * * Can be freely distributed and used under the terms of the GNU GPL. */ @@ -13,7 +13,7 @@ typedef struct birdsock { resource r; - pool *pool; /* Pool for socket data */ + pool *pool; /* Pool where incoming connections should be allocated (for SK_xxx_PASSIVE) */ int type; /* Socket type */ void *data; /* User data */ ip_addr saddr, daddr; /* IPA_NONE = unspecified */ @@ -38,12 +38,11 @@ typedef struct birdsock { int fd; /* System-dependent data */ node n; - int entered; + void *rbuf_alloc, *tbuf_alloc; } sock; sock *sk_new(pool *); /* Allocate new socket */ int sk_open(sock *); /* Open socket */ -void sk_close(sock *); /* Safe close of socket even from socket hook */ int sk_send(sock *, unsigned len); /* Send data, <0=err, >0=ok, 0=sleep */ int sk_send_to(sock *, unsigned len, ip_addr to, unsigned port); /* sk_send to given destination */ void sk_dump_all(void); @@ -68,7 +67,6 @@ sk_send_buffer_empty(sock *sk) #define SK_MAGIC 7 /* Internal use by sysdep code */ #define SK_UNIX_PASSIVE 8 #define SK_UNIX 9 -#define SK_DELETED 10 /* Internal use by sk_close */ /* * Multicast sockets are slightly different from the other ones: diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c index 3d70192c..b8e2abbc 100644 --- a/proto/bgp/bgp.c +++ b/proto/bgp/bgp.c @@ -139,7 +139,7 @@ bgp_close_conn(struct bgp_conn *conn) conn->keepalive_timer = NULL; rfree(conn->hold_timer); conn->hold_timer = NULL; - sk_close(conn->sk); + rfree(conn->sk); conn->sk = NULL; conn->state = BS_IDLE; if (conn->error_flag > 1) @@ -235,7 +235,7 @@ bgp_sock_err(sock *sk, int err) { case BS_CONNECT: case BS_OPENSENT: - sk_close(conn->sk); + rfree(conn->sk); conn->sk = NULL; conn->state = BS_ACTIVE; bgp_start_timer(conn->connect_retry_timer, p->cf->connect_retry_time); diff --git a/sysdep/unix/io.c b/sysdep/unix/io.c index f667801f..c3bd5f8c 100644 --- a/sysdep/unix/io.c +++ b/sysdep/unix/io.c @@ -1,7 +1,7 @@ /* * BIRD Internet Routing Daemon -- Unix I/O * - * (c) 1998--2000 Martin Mares + * (c) 1998--2004 Martin Mares * (c) 2004 Ondrej Filip * * Can be freely distributed and used under the terms of the GNU GPL. @@ -30,7 +30,6 @@ #include "lib/unix.h" #include "lib/sysio.h" -#define LOCAL_DEBUG /* * Tracked Files */ @@ -394,7 +393,7 @@ tm_format_reltime(char *x, bird_clock_t t) * (@rx_hook), when the contents of the transmit buffer have been transmitted * (@tx_hook) and when an error or connection close occurs (@err_hook). * - * You should not use rfree() from inside a socket hook, please use sk_close() instead. + * Freeing of sockets from inside socket hooks is perfectly safe. */ #ifndef SOL_IP @@ -410,16 +409,34 @@ tm_format_reltime(char *x, bird_clock_t t) #endif static list sock_list; +static struct birdsock *current_sock; +static int sock_recalc_fdsets_p; + +static inline sock * +sk_next(sock *s) +{ + if (!s->n.next->next) + return NULL; + else + return SKIP_BACK(sock, n, s->n.next); +} static void sk_free(resource *r) { sock *s = (sock *) r; + if (s->rbuf_alloc) + xfree(s->rbuf_alloc); + if (s->tbuf_alloc) + xfree(s->tbuf_alloc); if (s->fd >= 0) { close(s->fd); + if (s == current_sock) + current_sock = sk_next(s); rem_node(&s->n); + sock_recalc_fdsets_p = 1; } } @@ -474,12 +491,16 @@ sk_new(pool *p) s->tbsize = 0; s->err_hook = NULL; s->fd = -1; - s->entered = 0; + s->rbuf_alloc = s->tbuf_alloc = NULL; return s; } -#define ERR(x) do { err = x; goto bad; } while(0) -#define WARN(x) log(L_WARN "sk_setup: %s: %m", x) +static void +sk_insert(sock *s) +{ + add_tail(&sock_list, &s->n); + sock_recalc_fdsets_p = 1; +} #ifdef IPV6 @@ -534,6 +555,9 @@ get_sockaddr(struct sockaddr_in *sa, ip_addr *a, unsigned *port, int check) #endif +#define ERR(x) do { err = x; goto bad; } while(0) +#define WARN(x) log(L_WARN "sk_setup: %s: %m", x) + static char * sk_setup(sock *s) { @@ -566,10 +590,10 @@ static void sk_alloc_bufs(sock *s) { if (!s->rbuf && s->rbsize) - s->rbuf = mb_alloc(s->pool, s->rbsize); + s->rbuf = s->rbuf_alloc = xmalloc(s->rbsize); s->rpos = s->rbuf; if (!s->tbuf && s->tbsize) - s->tbuf = mb_alloc(s->pool, s->tbsize); + s->tbuf = s->tbuf_alloc = xmalloc(s->tbsize); s->tpos = s->ttx = s->tbuf; } @@ -597,7 +621,7 @@ sk_passive_connected(sock *s, struct sockaddr *sa, int al, int type) t->tbsize = s->tbsize; if (type == SK_TCP) get_sockaddr((sockaddr *) sa, &t->daddr, &t->dport, 1); - add_tail(&sock_list, &t->n); + sk_insert(t); if (err = sk_setup(t)) { log(L_ERR "Incoming connection: %s: %m", err); @@ -663,9 +687,8 @@ sk_open(sock *s) s->fd = fd; if (err = sk_setup(s)) - { goto bad; - } + switch (type) { case SK_UDP: @@ -769,7 +792,7 @@ sk_open(sock *s) #endif } - add_tail(&sock_list, &s->n); + sk_insert(s); return 0; bad: @@ -799,7 +822,7 @@ sk_open_unix(sock *s, char *name) ERR("bind"); if (listen(fd, 8)) ERR("listen"); - add_tail(&sock_list, &s->n); + sk_insert(s); return 0; bad: @@ -809,23 +832,6 @@ bad: return -1; } -/** - * sk_close - close a socket - * @s: a socket - * - * If sk_close() has been called from outside of any socket hook, - * it translates to a rfree(), else it just marks the socket for - * deletion as soon as the socket hook returns. - */ -void -sk_close(sock *s) -{ - if (s && s->entered) - s->type = SK_DELETED; - else - rfree(s); -} - static int sk_maybe_write(sock *s) { @@ -954,15 +960,17 @@ sk_read(sock *s) { s->rpos += c; if (s->rx_hook(s, s->rpos - s->rbuf)) - s->rpos = s->rbuf; + { + /* We need to be careful since the socket could have been deleted by the hook */ + if (current_sock == s) + s->rpos = s->rbuf; + } return 1; } return 0; } case SK_MAGIC: return s->rx_hook(s, 0); - case SK_DELETED: - return 0; default: { sockaddr sa; @@ -983,7 +991,7 @@ sk_read(sock *s) } } -static void +static int sk_write(sock *s) { switch (s->type) @@ -996,13 +1004,15 @@ sk_write(sock *s) sk_tcp_connected(s); else if (errno != EINTR && errno != EAGAIN && errno != EINPROGRESS) s->err_hook(s, errno); - break; + return 0; } - case SK_DELETED: - return; default: - while (s->ttx != s->tpos && sk_maybe_write(s) > 0) - s->tx_hook(s); + if (s->ttx != s->tpos && sk_maybe_write(s) > 0) + { + s->tx_hook(s); + return 1; + } + return 0; } } @@ -1052,10 +1062,9 @@ io_loop(void) time_t tout; int hi, events; sock *s; - node *n, *p; + node *n; - FD_ZERO(&rd); - FD_ZERO(&wr); + sock_recalc_fdsets_p = 1; for(;;) { events = ev_run_list(&global_event_list); @@ -1069,6 +1078,13 @@ io_loop(void) timo.tv_sec = events ? 0 : tout - now; timo.tv_usec = 0; + if (sock_recalc_fdsets_p) + { + sock_recalc_fdsets_p = 0; + FD_ZERO(&rd); + FD_ZERO(&wr); + } + hi = 0; WALK_LIST(n, sock_list) { @@ -1079,12 +1095,16 @@ io_loop(void) if (s->fd > hi) hi = s->fd; } + else + FD_CLR(s->fd, &rd); if (s->tx_hook && s->ttx != s->tpos) { FD_SET(s->fd, &wr); if (s->fd > hi) hi = s->fd; } + else + FD_CLR(s->fd, &wr); } /* @@ -1122,24 +1142,29 @@ io_loop(void) } if (hi) { - WALK_LIST_DELSAFE(n, p, sock_list) + current_sock = SKIP_BACK(sock, n, HEAD(sock_list)); /* guaranteed to be non-empty */ + while (current_sock) { - s = SKIP_BACK(sock, n, n); - s->entered = 1; + sock *s = current_sock; + int e; if (FD_ISSET(s->fd, &rd)) - { - FD_CLR(s->fd, &rd); - while (sk_read(s)) - ; - } - if (s->type != SK_DELETED && FD_ISSET(s->fd, &wr)) - { - FD_CLR(s->fd, &wr); - sk_write(s); - } - s->entered = 0; - if (s->type == SK_DELETED) - rfree(s); + do + { + e = sk_read(s); + if (s != current_sock) + goto next; + } + while (e); + if (FD_ISSET(s->fd, &wr)) + do + { + e = sk_write(s); + if (s != current_sock) + goto next; + } + while (e); + current_sock = sk_next(s); + next: ; } } } diff --git a/sysdep/unix/main.c b/sysdep/unix/main.c index 75852c18..2479cd6a 100644 --- a/sysdep/unix/main.c +++ b/sysdep/unix/main.c @@ -249,7 +249,7 @@ cli_err(sock *s, int err) log(L_INFO "CLI connection closed"); } cli_free(s->data); - sk_close(s); + rfree(s); } static int