sr-dev

sr-dev@lists.kamailio.org

44307 discussions

git:kamailio_3.0: core: fix selects in optimized rvalue expressions

by Andrei Pelinescu-Onciul

Module: sip-router Branch: kamailio_3.0 Commit: 1ee403b74de46a14a0c9a8c2e507160c182230b1 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=1ee403b… Author: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Date: Fri Jun 18 22:22:59 2010 +0200 core: fix selects in optimized rvalue expressions - do not attempt to resolve_select() twice the same select. This happened when a rve containing only a select was optimized, the select moved out of the rve and fix_expr() tried to "fix" the already resolved select later. - changed BUG() messages for failed resolve_select() into ERR(). (cherry picked from commit a7fa13ed757d74cfd4486f3c282ec50259929edc) --- route.c | 20 ++++++++++++++------ route_struct.h | 5 +++-- rvalue.c | 2 +- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/route.c b/route.c index 13608a2..9d9e94d 100644 --- a/route.c +++ b/route.c @@ -580,7 +580,9 @@ int fix_expr(struct expr* exp) exp->r.re=re; exp->r_type=RE_ST; }else if (exp->r_type!=RE_ST && exp->r_type != AVP_ST - && exp->r_type != SELECT_ST && exp->r_type!= RVE_ST + && exp->r_type != SELECT_ST && + exp->r_type != SELECT_UNFIXED_ST && + exp->r_type!= RVE_ST && exp->r_type != PVAR_ST){ LOG(L_CRIT, "BUG: fix_expr : invalid type for match\n"); return E_BUG; @@ -593,19 +595,21 @@ int fix_expr(struct expr* exp) return ret; } } - if (exp->l_type==SELECT_O) { + if (exp->l_type==SELECT_UNFIXED_O) { if ((ret=resolve_select(exp->l.select)) < 0) { - BUG("Unable to resolve select\n"); + ERR("Unable to resolve select\n"); print_select(exp->l.select); return ret; } + exp->l_type=SELECT_O; } - if ((exp->r_type==SELECT_O)||(exp->r_type==SELECT_ST)) { + if (exp->r_type==SELECT_UNFIXED_ST) { if ((ret=resolve_select(exp->r.select)) < 0) { - BUG("Unable to resolve select\n"); - print_select(exp->l.select); + ERR("Unable to resolve select\n"); + print_select(exp->r.select); return ret; } + exp->r_type=SELECT_ST; } /* PVAR don't need fixing */ ret=0; @@ -1745,6 +1749,10 @@ inline static int eval_elem(struct run_act_ctx* h, struct expr* e, case PVAR_O: ret=comp_pvar(e->op, e->l.param, e->r_type, &e->r, msg, h); break; + + case SELECT_UNFIXED_O: + BUG("unexpected unfixed select operand %d\n", e->l_type); + break; /* default: LOG(L_CRIT, "BUG: eval_elem: invalid operand %d\n", diff --git a/route_struct.h b/route_struct.h index 4894bd5..3f7e854 100644 --- a/route_struct.h +++ b/route_struct.h @@ -76,7 +76,7 @@ enum _expr_l_type{ METHOD_O=51, URI_O, FROM_URI_O, TO_URI_O, SRCIP_O, SRCPORT_O, DSTIP_O, DSTPORT_O, PROTO_O, AF_O, MSGLEN_O, ACTION_O, NUMBER_O, AVP_O, SNDIP_O, SNDPORT_O, TOIP_O, TOPORT_O, SNDPROTO_O, - SNDAF_O, RETCODE_O, SELECT_O, PVAR_O, RVEXP_O}; + SNDAF_O, RETCODE_O, SELECT_O, PVAR_O, RVEXP_O, SELECT_UNFIXED_O}; /* action types */ enum action_type{ FORWARD_T=1, SEND_T, DROP_T, LOG_T, ERROR_T, ROUTE_T, EXEC_T, @@ -121,7 +121,8 @@ enum _operand_subtype{ SELECT_ST, PVAR_ST, LVAL_ST, RVE_ST, RETCODE_ST, CASE_ST, - BLOCK_ST, JUMPTABLE_ST, CONDTABLE_ST, MATCH_CONDTABLE_ST + BLOCK_ST, JUMPTABLE_ST, CONDTABLE_ST, MATCH_CONDTABLE_ST, + SELECT_UNFIXED_ST }; typedef enum _expr_l_type expr_l_type; diff --git a/rvalue.c b/rvalue.c index 22a7b2b..b17e664 100644 --- a/rvalue.c +++ b/rvalue.c @@ -2761,7 +2761,7 @@ static int fix_rval(struct rvalue* rv) return fix_actions(rv->v.action); case RV_SEL: if (resolve_select(&rv->v.sel)<0){ - BUG("Unable to resolve select\n"); + ERR("Unable to resolve select\n"); print_select(&rv->v.sel); } return 0;

14 years, 5 months

git:kamailio_3.0: sercmd: fix cfg. var name completion

by Andrei Pelinescu-Onciul

Module: sip-router Branch: kamailio_3.0 Commit: 089c773b62311b93154518bbfd8d94cf4150c092 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=089c773… Author: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Date: Sun Aug 8 18:09:17 2010 +0200 sercmd: fix cfg. var name completion In some rare cases cfg. variable name completion did not work properly (e.g. reusing a command from history, editing it, changing the group and attempting command completion again). This was due to an assumption that the state would be re-initialized on new commands (which is mostly not true). (cherry picked from commit 261d8282e6dabf73a65904e7a8413ccec9faf2fd) --- utils/sercmd/sercmd.c | 49 ++++++++++++++++++++++--------------------------- 1 files changed, 22 insertions(+), 27 deletions(-) diff --git a/utils/sercmd/sercmd.c b/utils/sercmd/sercmd.c index 1f1a721..3b4432a 100644 --- a/utils/sercmd/sercmd.c +++ b/utils/sercmd/sercmd.c @@ -1802,36 +1802,31 @@ char** sercmd_completion(const char* text, int start, int end) } } #endif /* USE_MI */ -#ifdef USE_CFG_VARS }else if (crt_param_no==2){ - if (attempted_completion_state!=COMPLETE_CFG_GRP){ - for(i=0; complete_params_cfg_var[i]; i++){ - if ((cmd_len==strlen(complete_params_cfg_var[i])) && - (strncmp(&rl_line_buffer[cmd_start], +#ifdef USE_CFG_VARS + /* see if we complete cfg. var names for this command */ + for(i=0; complete_params_cfg_var[i]; i++){ + if ((cmd_len==strlen(complete_params_cfg_var[i])) && + (strncmp(&rl_line_buffer[cmd_start], complete_params_cfg_var[i], cmd_len)==0)){ - attempted_completion_state=COMPLETE_CFG_GRP; - /* find grp_start */ - for(j=cmd_end; (j<start) && ((rl_line_buffer[j]==' ') - || (rl_line_buffer[j]=='\t')); j++); - grp_start=j; - break; - } - } - } - if (attempted_completion_state==COMPLETE_CFG_GRP){ - /* get the group name from the grp_param */ - /* find first whitespace for the group name*/ - for(j=grp_start; (j<start) && (rl_line_buffer[j]!=' ') && - (rl_line_buffer[j]!='\t'); j++); - grp_len=j-grp_start; - for(grp=cfg_grp_lst; grp; grp=grp->next){ - if (grp_len==grp->grp_name.len && - memcmp(&rl_line_buffer[grp_start], grp->grp_name.s, - grp_len)==0) { - attempted_completion_state=COMPLETE_CFG_VAR; - crt_cfg_grp=grp; - goto end; + /* get the group name: */ + /* find grp_start */ + for(j=cmd_end; (j<start) && ((rl_line_buffer[j]==' ') || + (rl_line_buffer[j]=='\t')); j++); + grp_start=j; + /* find group end / grp_len*/ + for(j=grp_start; (j<start) && (rl_line_buffer[j]!=' ') && + (rl_line_buffer[j]!='\t'); j++); + grp_len=j-grp_start; + for(grp=cfg_grp_lst; grp; grp=grp->next){ + if (grp_len==grp->grp_name.len && + memcmp(&rl_line_buffer[grp_start], + grp->grp_name.s, grp_len)==0) { + attempted_completion_state=COMPLETE_CFG_VAR; + crt_cfg_grp=grp; + goto end; + } } } }

14 years, 5 months

git:kamailio_3.0: tcp: fix fd passing bug

by Andrei Pelinescu-Onciul

Module: sip-router Branch: kamailio_3.0 Commit: 66619631879132e3d93fd858fa81c79ac9788617 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=6661963… Author: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Date: Wed Jun 16 21:03:06 2010 +0200 tcp: fix fd passing bug If connections are opened and closed very quickly when data is sent on them, it is possible that a connection gets closed (close() inside tcp_main) while a process waits for its fd, just before tcp_main attempts to send the fd. In this case the fd sending will fail (one cannot send a closed fd) and the process that asked for it will remain waiting forever. The fix always checks before attempting to send the fd if the fd is still open and the connection is not in a "bad" state. If not, a new error response is sent (no fd and connection == NULL). (backported from 9da6fae72b9883ab8dbbb4e681c4d4e96d6549e4) --- tcp_main.c | 51 +++++++++++++++++++++++++++++++++++++++++++++------ 1 files changed, 45 insertions(+), 6 deletions(-) diff --git a/tcp_main.c b/tcp_main.c index 4840ddc..6a6c4c0 100644 --- a/tcp_main.c +++ b/tcp_main.c @@ -2050,14 +2050,25 @@ int tcp_send(struct dest_info* dst, union sockaddr_union* from, do_close_fd=0; goto release_c; } - if (unlikely(c!=tmp)){ - LOG(L_CRIT, "BUG: tcp_send: get_fd: got different connection:" + /* handle fd closed or bad connection/error + (it's possible that this happened in the time between + we found the intial connection and the time when we get + the fd) + */ + if (unlikely(c!=tmp || fd==-1 || c->state==S_CONN_BAD)){ + if (unlikely(c!=tmp && tmp!=0)) + BUG("tcp_send: get_fd: got different connection:" " %p (id= %d, refcnt=%d state=%d) != " " %p (n=%d)\n", c, c->id, atomic_get(&c->refcnt), c->state, tmp, n - ); + ); n=-1; /* fail */ + /* don't cache fd & close it */ + do_close_fd = (fd==-1)?0:1; +#ifdef TCP_FD_CACHE + use_fd_cache = 0; +#endif /* TCP_FD_CACHE */ goto end; } DBG("tcp_send: after receive_fd: c= %p n=%d fd=%d\n",c, n, fd); @@ -2721,6 +2732,12 @@ inline static void send_fd_queue_run(struct tcp_send_fd_q* q) struct send_fd_info* t; for (p=t=&q->data[0]; p<q->crt; p++){ + if (unlikely(p->tcp_conn->state == S_CONN_BAD || + p->tcp_conn->flags & F_CONN_FD_CLOSED || + p->tcp_conn->s ==-1)) { + /* bad and/or already closed connection => remove */ + goto rm_con; + } if (unlikely(send_fd(p->unix_sock, &(p->tcp_conn), sizeof(struct tcp_connection*), p->tcp_conn->s)<=0)){ if ( ((errno==EAGAIN)||(errno==EWOULDBLOCK)) && @@ -2736,7 +2753,11 @@ inline static void send_fd_queue_run(struct tcp_send_fd_q* q) p->unix_sock, (long)(p-&q->data[0]), p->retries, p->tcp_conn, p->tcp_conn->s, errno, strerror(errno)); +rm_con: #ifdef TCP_ASYNC + /* if a connection is on the send_fd queue it means it's + not watched for read anymore => could be watched only for + write */ if (p->tcp_conn->flags & F_CONN_WRITE_W){ io_watch_del(&io_h, p->tcp_conn->s, -1, IO_FD_CLOSING); p->tcp_conn->flags &=~F_CONN_WRITE_W; @@ -3006,6 +3027,7 @@ error: inline static int handle_ser_child(struct process_table* p, int fd_i) { struct tcp_connection* tcpconn; + struct tcp_connection* tmp; long response[2]; int cmd; int bytes; @@ -3101,9 +3123,26 @@ inline static int handle_ser_child(struct process_table* p, int fd_i) /* send the requested FD */ /* WARNING: take care of setting refcnt properly to * avoid race conditions */ - if (unlikely(send_fd(p->unix_sock, &tcpconn, sizeof(tcpconn), - tcpconn->s)<=0)){ - LOG(L_ERR, "ERROR: handle_ser_child: send_fd failed\n"); + if (unlikely(tcpconn->state == S_CONN_BAD || + (tcpconn->flags & F_CONN_FD_CLOSED) || + tcpconn->s ==-1)) { + /* connection is already marked as bad and/or has no + fd => don't try to send the fd (trying to send a + closed fd _will_ fail) */ + tmp = 0; + if (unlikely(send_all(p->unix_sock, &tmp, sizeof(tmp)) <= 0)) + BUG("handle_ser_child: CONN_GET_FD: send_all failed\n"); + /* no need to attempt to destroy the connection, it should + be already in the process of being destroyed */ + } else if (unlikely(send_fd(p->unix_sock, &tcpconn, + sizeof(tcpconn), tcpconn->s)<=0)){ + LOG(L_ERR, "handle_ser_child: CONN_GET_FD:" + " send_fd failed\n"); + /* try sending error (better then not sending anything) */ + tmp = 0; + if (unlikely(send_all(p->unix_sock, &tmp, sizeof(tmp)) <= 0)) + BUG("handle_ser_child: CONN_GET_FD:" + " send_fd send_all fallback failed\n"); } break; case CONN_NEW:

14 years, 5 months

git:kamailio_3.0: tcp: fix dispatching closed connections to tcp readers

by Andrei Pelinescu-Onciul

Module: sip-router Branch: kamailio_3.0 Commit: a160156bdca1708bcbca7e000c2da91c13f67336 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=a160156… Author: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Date: Wed Jul 7 11:59:30 2010 +0200 tcp: fix dispatching closed connections to tcp readers Under very heavy load it is possible that send2child() might try to send an already closed connection/fd to a tcp reader. This can happen only if the tcp connection is watched for read (POLLIN) by tcp_main (and not by a tcp reader), the connection becomes available for reading (either new data received, EOF or RST) and tcp_main chooses a specific tcp reader to send the connection to while in the same time the same tcp reader tries to send on the same connection, fails for some reason (no more space for buffering, EOF, RST a.s.o) and sends a close command back to tcp_main. Because send2child() executes first any pending commands from the choosen tcp_reader, this might lead to closing the fd before attempting to send it to the tcp_reader. Under normal circumstances the impact is only an extra syscall and some log messages, however it is possible (but highly unlikely) that after sending the close command the tcp_reader opens a new connection for sending and sends its fd back to tcp_main. This new fd might get the same number as the freshly closed fd and send2child might send the wrong (fd, tcp connection) pair. (cherry picked from commit d89437a3d7bc25a9c098a04c6ee69fc3848ff0b5) --- tcp_main.c | 20 ++++++++++++++------ 1 files changed, 14 insertions(+), 6 deletions(-) diff --git a/tcp_main.c b/tcp_main.c index 6a6c4c0..54334ab 100644 --- a/tcp_main.c +++ b/tcp_main.c @@ -2468,6 +2468,7 @@ close_again: LOG(L_ERR, "ERROR: tcpconn_put_destroy; close() failed: %s (%d)\n", strerror(errno), errno); } + tcpconn->s=-1; } @@ -3378,10 +3379,20 @@ inline static int send2child(struct tcp_connection* tcpconn) * send a release command, but the master fills its socket buffer * with new connection commands => deadlock) */ /* answer tcp_send requests first */ - while(handle_ser_child(&pt[tcp_children[idx].proc_no], -1)>0); + while(unlikely((tcpconn->state != S_CONN_BAD) && + (handle_ser_child(&pt[tcp_children[idx].proc_no], -1)>0))); /* process tcp readers requests */ - while(handle_tcp_child(&tcp_children[idx], -1)>0); - + while(unlikely((tcpconn->state != S_CONN_BAD && + (handle_tcp_child(&tcp_children[idx], -1)>0)))); + + /* the above possible pending requests might have included a + command to close this tcpconn (e.g. CONN_ERROR, CONN_EOF). + In this case the fd is already closed here (and possible + even replaced by another one with the same number) so it + must not be sent to a reader anymore */ + if (unlikely(tcpconn->state == S_CONN_BAD || + (tcpconn->flags & F_CONN_FD_CLOSED))) + return -1; #ifdef SEND_FD_QUEUE /* if queue full, try to queue the io */ if (unlikely(send_fd(tcp_children[idx].unix_sock, &tcpconn, @@ -3501,8 +3512,6 @@ static inline int handle_new_connect(struct socket_info* si) DBG("handle_new_connect: new connection from %s: %p %d flags: %04x\n", su2a(&su, sizeof(su)), tcpconn, tcpconn->s, tcpconn->flags); if(unlikely(send2child(tcpconn)<0)){ - LOG(L_ERR,"ERROR: handle_new_connect: no children " - "available\n"); tcpconn->flags&=~F_CONN_READER; tcpconn_put(tcpconn); tcpconn_try_unhash(tcpconn); @@ -3676,7 +3685,6 @@ send_to_child: tcpconn->flags&=~(F_CONN_MAIN_TIMER|F_CONN_READ_W|F_CONN_WANTS_RD); tcpconn_ref(tcpconn); /* refcnt ++ */ if (unlikely(send2child(tcpconn)<0)){ - LOG(L_ERR,"ERROR: handle_tcpconn_ev: no children available\n"); tcpconn->flags&=~F_CONN_READER; #ifdef TCP_ASYNC if (tcpconn->flags & F_CONN_WRITE_W){

14 years, 5 months

git:kamailio_3.0: tcp: force eof after read if write side hangup

by Andrei Pelinescu-Onciul

Module: sip-router Branch: kamailio_3.0 Commit: 4ca0f2295fcb2b93a853edad472c5578b335e72a URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=4ca0f22… Author: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Date: Sun Jun 20 19:40:55 2010 +0200 tcp: force eof after read if write side hangup Even if POLLRDHUP is not supported, but we detected a write side close (POLLHUP) or an error (POLLERR) or such an event was previously detected by tcp_main (F_CONN_EOF_SEEN), force connection closing after reading all the data in the socket buffer. In this case we can close() after the first short read and we save an extra system call (a read() that returns 0). (cherry picked from commit 28e313250503d6f8d06ebab15c8421c40e7f0fe4) --- tcp_read.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tcp_read.c b/tcp_read.c index d201765..9015a8b 100644 --- a/tcp_read.c +++ b/tcp_read.c @@ -970,13 +970,13 @@ again: con, con->id, atomic_get(&con->refcnt)); goto read_error; } + read_flags=(( #ifdef POLLRDHUP - read_flags=(((events & POLLRDHUP) | + (events & POLLRDHUP) | +#endif /* POLLRDHUP */ + (events & (POLLHUP|POLLERR)) | (con->flags & (F_CONN_EOF_SEEN|F_CONN_FORCE_EOF))) && !(events & POLLPRI))? RD_CONN_FORCE_EOF: 0; -#else /* POLLRDHUP */ - read_flags=0; -#endif /* POLLRDHUP */ resp=tcp_read_req(con, &ret, &read_flags); if (unlikely(resp<0)){ read_error:

14 years, 5 months

git:kamailio_3.0: tcp: more consistent IO_FD_CLOSING usage

by Andrei Pelinescu-Onciul

Module: sip-router Branch: kamailio_3.0 Commit: 5b30df2dc48ce9bb31b88b523fc54ad8292e3251 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=5b30df2… Author: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Date: Sat Jun 19 00:21:58 2010 +0200 tcp: more consistent IO_FD_CLOSING usage - extra safety checks before using IO_FD_CLOSING in io_watch_del() calls (must be used only when the fd will be close() imediately afterwards). - add IO_FD_CLOSING when POLLERR or POLLHUP is detected in tcp_main and the socket receive buffer is empty (in this case the fd will be shortly closed) (cherry picked from commit 504ef98ed34366303476032013c28a4a7d1c8131) --- tcp_main.c | 79 ++++++++++++++++++++++++++++++++++++++++++++--------------- 1 files changed, 59 insertions(+), 20 deletions(-) diff --git a/tcp_main.c b/tcp_main.c index 54334ab..255753d 100644 --- a/tcp_main.c +++ b/tcp_main.c @@ -2889,18 +2889,31 @@ inline static int handle_tcp_child(struct tcp_child* tcp_c, int fd_i) case CONN_RELEASE: tcp_c->busy--; if (unlikely(tcpconn_put(tcpconn))){ + /* if refcnt was 1 => it was used only in the + tcp reader => it's not hashed or watched for IO + anymore => no need to io_watch_del() */ tcpconn_destroy(tcpconn); break; } if (unlikely(tcpconn->state==S_CONN_BAD)){ + if (tcpconn_try_unhash(tcpconn)) { #ifdef TCP_ASYNC - if (unlikely(tcpconn->flags & F_CONN_WRITE_W)){ - io_watch_del(&io_h, tcpconn->s, -1, IO_FD_CLOSING); + if (unlikely(tcpconn->flags & F_CONN_WRITE_W)){ + io_watch_del(&io_h, tcpconn->s, -1, IO_FD_CLOSING); + tcpconn->flags &= ~F_CONN_WRITE_W; + } +#endif /* TCP_ASYNC */ + tcpconn_put_destroy(tcpconn); + } +#ifdef TCP_ASYNC + else if (unlikely(tcpconn->flags & F_CONN_WRITE_W)){ + /* should never happen: if it's already unhashed, it + should not be watched for IO */ + BUG("unhashed connection watched for write\n"); + io_watch_del(&io_h, tcpconn->s, -1, 0); tcpconn->flags &= ~F_CONN_WRITE_W; } #endif /* TCP_ASYNC */ - if (tcpconn_try_unhash(tcpconn)) - tcpconn_put_destroy(tcpconn); break; } /* update the timeout*/ @@ -2937,12 +2950,17 @@ inline static int handle_tcp_child(struct tcp_child* tcp_c, int fd_i) TCP_EV_SEND_TIMEOUT(0, &tcpconn->rcv); TCP_STATS_SEND_TIMEOUT(); } - if (unlikely(tcpconn->flags & F_CONN_WRITE_W)){ - io_watch_del(&io_h, tcpconn->s, -1, IO_FD_CLOSING); + if (tcpconn_try_unhash(tcpconn)) { + if (unlikely(tcpconn->flags & F_CONN_WRITE_W)){ + io_watch_del(&io_h, tcpconn->s, -1, IO_FD_CLOSING); + tcpconn->flags&=~F_CONN_WRITE_W; + } + tcpconn_put_destroy(tcpconn); + } else if (unlikely(tcpconn->flags & F_CONN_WRITE_W)){ + BUG("unhashed connection watched for write\n"); + io_watch_del(&io_h, tcpconn->s, -1, 0); tcpconn->flags&=~F_CONN_WRITE_W; } - if (tcpconn_try_unhash(tcpconn)) - tcpconn_put_destroy(tcpconn); break; }else{ crt_timeout=MIN_unsigned(con_lifetime, @@ -2967,14 +2985,22 @@ inline static int handle_tcp_child(struct tcp_child* tcp_c, int fd_i) LOG(L_CRIT, "ERROR: tcp_main: handle_tcp_child: failed to add" " new socket to the fd list\n"); tcpconn->flags&=~F_CONN_READ_W; + if (tcpconn_try_unhash(tcpconn)) { #ifdef TCP_ASYNC - if (unlikely(tcpconn->flags & F_CONN_WRITE_W)){ - io_watch_del(&io_h, tcpconn->s, -1, IO_FD_CLOSING); + if (unlikely(tcpconn->flags & F_CONN_WRITE_W)){ + io_watch_del(&io_h, tcpconn->s, -1, IO_FD_CLOSING); + tcpconn->flags&=~F_CONN_WRITE_W; + } +#endif /* TCP_ASYNC */ + tcpconn_put_destroy(tcpconn); + } +#ifdef TCP_ASYNC + else if (unlikely(tcpconn->flags & F_CONN_WRITE_W)) { + BUG("unhashed connection watched for write\n"); + io_watch_del(&io_h, tcpconn->s, -1, 0); tcpconn->flags&=~F_CONN_WRITE_W; } #endif /* TCP_ASYNC */ - if (tcpconn_try_unhash(tcpconn)) - tcpconn_put_destroy(tcpconn); break; } DBG("handle_tcp_child: CONN_RELEASE %p refcnt= %d\n", @@ -3240,10 +3266,16 @@ inline static int handle_ser_child(struct process_table* p, int fd_i) POLLIN|POLLOUT, -1)<0)){ LOG(L_CRIT, "ERROR: tcp_main: handle_ser_child:" " failed to change socket watch events\n"); - io_watch_del(&io_h, tcpconn->s, -1, IO_FD_CLOSING); - tcpconn->flags&=~F_CONN_READ_W; - if (tcpconn_try_unhash(tcpconn)) + if (tcpconn_try_unhash(tcpconn)) { + io_watch_del(&io_h, tcpconn->s, -1, + IO_FD_CLOSING); + tcpconn->flags&=~F_CONN_READ_W; tcpconn_put_destroy(tcpconn); + } else { + BUG("unhashed connection watched for IO\n"); + io_watch_del(&io_h, tcpconn->s, -1, 0); + tcpconn->flags&=~F_CONN_READ_W; + } break; } } @@ -3573,10 +3605,6 @@ inline static int handle_tcpconn_ev(struct tcp_connection* tcpconn, short ev, (wbufq_run(tcpconn->s, tcpconn, &empty_q)<0) || (empty_q && tcpconn_close_after_send(tcpconn)) )){ - if (unlikely(io_watch_del(&io_h, tcpconn->s, fd_i, 0)<0)){ - LOG(L_ERR, "ERROR: handle_tcpconn_ev: io_watch_del(1) failed:" - " for %p, fd %d\n", tcpconn, tcpconn->s); - } if ((tcpconn->flags & F_CONN_READ_W) && (ev & POLLIN)){ /* connection is watched for read and there is a read event * (unfortunately if we have POLLIN here we don't know if @@ -3589,6 +3617,11 @@ inline static int handle_tcpconn_ev(struct tcp_connection* tcpconn, short ev, * conn. to a a child only if needed (another syscall + at * least 2 * syscalls in the reader + ...) */ if ((ioctl(tcpconn->s, FIONREAD, &bytes)>=0) && (bytes>0)){ + if (unlikely(io_watch_del(&io_h, tcpconn->s, fd_i, 0)<0)){ + LOG(L_ERR, "ERROR: handle_tcpconn_ev: io_watch_del(1)" + " failed: for %p, fd %d\n", + tcpconn, tcpconn->s); + } tcpconn->flags&=~(F_CONN_WRITE_W|F_CONN_READ_W| F_CONN_WANTS_RD|F_CONN_WANTS_WR); tcpconn->flags|=F_CONN_FORCE_EOF|F_CONN_WR_ERROR; @@ -3596,6 +3629,11 @@ inline static int handle_tcpconn_ev(struct tcp_connection* tcpconn, short ev, } /* if bytes==0 or ioctl failed, destroy the connection now */ } + if (unlikely(io_watch_del(&io_h, tcpconn->s, fd_i, + IO_FD_CLOSING) < 0)){ + LOG(L_ERR, "ERROR: handle_tcpconn_ev: io_watch_del() failed:" + " for %p, fd %d\n", tcpconn, tcpconn->s); + } tcpconn->flags&=~(F_CONN_WRITE_W|F_CONN_READ_W| F_CONN_WANTS_RD|F_CONN_WANTS_WR); if (unlikely(ev & POLLERR)){ @@ -3688,7 +3726,8 @@ send_to_child: tcpconn->flags&=~F_CONN_READER; #ifdef TCP_ASYNC if (tcpconn->flags & F_CONN_WRITE_W){ - if (unlikely(io_watch_del(&io_h, tcpconn->s, fd_i, 0)<0)){ + if (unlikely(io_watch_del(&io_h, tcpconn->s, fd_i, + IO_FD_CLOSING) < 0)){ LOG(L_ERR, "ERROR: handle_tcpconn_ev: io_watch_del(4)" " failed:" " for %p, fd %d\n", tcpconn, tcpconn->s);

14 years, 5 months

git:kamailio_3.0: io_wait: don't update FD watched status on error

by Andrei Pelinescu-Onciul

Module: sip-router Branch: kamailio_3.0 Commit: bf75177d709d97d70db9ca21d4f9526bd50fbcf5 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=bf75177… Author: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Date: Fri Jun 18 09:48:21 2010 +0200 io_wait: don't update FD watched status on error If the syscall to change the events or delete a watched FD fails, don't update/delete the FD status in fd_hash. For /dev/poll if a change fails when re-adding the FD, delete it from the hash (in the /dev/poll case to change the events a FD is watched for one has to remove it and re-add it with the new events). The syscalls should never fail in an un-handled way, but in the unlikely event that it happens this change will make the code more robust. (cherry picked from commit 2d8cd170ab867ab15296b30f0b784abe1adc1bca) --- io_wait.h | 15 +++++++++------ 1 files changed, 9 insertions(+), 6 deletions(-) diff --git a/io_wait.h b/io_wait.h index 93f1426..01df1e6 100644 --- a/io_wait.h +++ b/io_wait.h @@ -627,7 +627,6 @@ inline static int io_watch_del(io_wait_h* h, int fd, int idx, int flags) goto error; } events=e->events; - unhash_fd_map(e); switch(h->poll_method){ case POLL_POLL: @@ -647,7 +646,6 @@ inline static int io_watch_del(io_wait_h* h, int fd, int idx, int flags) #endif #ifdef HAVE_SIGIO_RT case POLL_SIGIO_RT: - fix_fd_array; /* the O_ASYNC flag must be reset all the time, the fd * can be changed only if O_ASYNC is reset (if not and * the fd is a duplicate, you will get signals from the dup. fd @@ -667,6 +665,7 @@ inline static int io_watch_del(io_wait_h* h, int fd, int idx, int flags) " failed: %s [%d]\n", strerror(errno), errno); goto error; } + fix_fd_array; /* only on success */ break; #endif #ifdef HAVE_EPOLL @@ -737,6 +736,7 @@ again_devpoll: h->poll_method); goto error; } + unhash_fd_map(e); /* only on success */ h->fd_no--; return 0; error: @@ -808,14 +808,14 @@ inline static int io_watch_chg(io_wait_h* h, int fd, short events, int idx ) add_events=events & ~e->events; del_events=e->events & ~events; - e->events=events; switch(h->poll_method){ case POLL_POLL: + fd_array_chg(events #ifdef POLLRDHUP - /* listen to POLLRDHUP by default (if POLLIN) */ - events|=((int)!(events & POLLIN) - 1) & POLLRDHUP; + /* listen to POLLRDHUP by default (if POLLIN) */ + | (((int)!(events & POLLIN) - 1) & POLLRDHUP) #endif /* POLLRDHUP */ - fd_array_chg(events); + ); break; #ifdef HAVE_SELECT case POLL_SELECT: @@ -921,6 +921,8 @@ again_devpoll2: LOG(L_ERR, "ERROR: io_watch_chg: re-adding fd to " "/dev/poll failed: %s [%d]\n", strerror(errno), errno); + /* error re-adding the fd => mark it as removed/unhash */ + unhash_fd_map(e); goto error; } break; @@ -931,6 +933,7 @@ again_devpoll2: h->poll_method); goto error; } + e->events=events; /* only on success */ return 0; error: return -1;

14 years, 5 months

git:kamailio_3.0: io_wait: fix kqueue io_wait_add & POLLIN

by Andrei Pelinescu-Onciul

Module: sip-router Branch: kamailio_3.0 Commit: 8e6609c4416dd4f1196daa793b75c305ca22155e URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=8e6609c… Author: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Date: Sat Jun 19 00:35:47 2010 +0200 io_wait: fix kqueue io_wait_add & POLLIN A "goto error" was placed outside the error handling "if", resulting in any io_watch_add(), that tried to enable write watching on a new FD, returning failure (fortunately this kind of io_watch_add() usage doesn't happen very often, usually write watch is enabled via io_watch_chg() on FDs already io_watch_add()'ed for reading). Only POLL_KQUEUE was affected by this bug, meaning the default on all *bsd and darwin. (cherry picked from commit e5be1a067158c8ba49d33082eb403937546e7c69) --- io_wait.h | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/io_wait.h b/io_wait.h index 01df1e6..b110b6b 100644 --- a/io_wait.h +++ b/io_wait.h @@ -499,7 +499,7 @@ again2: case POLL_KQUEUE: if (likely( events & POLLIN)){ if (unlikely(kq_ev_change(h, fd, EVFILT_READ, EV_ADD, e)==-1)) - goto error; + goto error; } if (unlikely( events & POLLOUT)){ if (unlikely(kq_ev_change(h, fd, EVFILT_WRITE, EV_ADD, e)==-1)) @@ -507,8 +507,8 @@ again2: if (likely(events & POLLIN)){ kq_ev_change(h, fd, EVFILT_READ, EV_DELETE, 0); } + goto error; } - goto error; } break; #endif

14 years, 5 months

git:kamailio_3.0: io_wait: fix kqueue and too many errors in changelist

by Andrei Pelinescu-Onciul

Module: sip-router Branch: kamailio_3.0 Commit: d3a1deaefdd0f23388fece48530a0ae1f39ad93a URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=d3a1dea… Author: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Date: Thu Jun 17 18:43:14 2010 +0200 io_wait: fix kqueue and too many errors in changelist kevent() tries to return errors in the changelist back in the supplied eventlist array. However if this is not large enough, the whole kevent() syscall will fail. Now if kevent() fails with EBADF the call will be retried with a smaller set of changes, until the entire original changelist is applied. Fixes also kq_ev_change() flush mode: on error it will try to apply the changes one-by-one. (this affects only systems that have kqueue: *bsd and darwin) (cherry picked from commit b0bd3201826ee693d6f96c6e336477b9d9db7c32) --- io_wait.h | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------ 1 files changed, 55 insertions(+), 7 deletions(-) diff --git a/io_wait.h b/io_wait.h index 44ef60c..93f1426 100644 --- a/io_wait.h +++ b/io_wait.h @@ -247,6 +247,7 @@ static inline int kq_ev_change(io_wait_h* h, int fd, int filter, int flag, void* data) { int n; + int r; struct timespec tspec; if (h->kq_nchanges>=h->kq_changes_size){ @@ -257,11 +258,36 @@ static inline int kq_ev_change(io_wait_h* h, int fd, int filter, int flag, tspec.tv_nsec=0; again: n=kevent(h->kq_fd, h->kq_changes, h->kq_nchanges, 0, 0, &tspec); - if (n==-1){ - if (errno==EINTR) goto again; - LOG(L_ERR, "ERROR: io_watch_add: kevent flush changes " + if (unlikely(n == -1)){ + if (likely(errno == EBADF)) { + /* one of the file descriptors is bad, probably already + closed => try to apply changes one-by-one */ + for (r = 0; r < h->kq_nchanges; r++) { +retry2: + n = kevent(h->kq_fd, &h->kq_changes[r], 1, 0, 0, &tspec); + if (n==-1) { + if (errno == EBADF) + continue; /* skip over it */ + if (errno == EINTR) + goto retry2; + LOG(L_ERR, "ERROR: io_watch_add: kevent flush changes" + " failed: %s [%d]\n", + strerror(errno), errno); + /* shift the array */ + memmove(&h->kq_changes[0], &h->kq_changes[r+1], + sizeof(h->kq_changes[0])* + (h->kq_nchanges-r-1)); + h->kq_nchanges-=(r+1); + return -1; + } + } + } else if (errno == EINTR) goto again; + else { + LOG(L_ERR, "ERROR: io_watch_add: kevent flush changes" " failed: %s [%d]\n", strerror(errno), errno); - return -1; + h->kq_nchanges=0; /* reset changes array */ + return -1; + } } h->kq_nchanges=0; /* changes array is empty */ } @@ -1076,22 +1102,43 @@ inline static int io_wait_loop_kqueue(io_wait_h* h, int t, int repeat) int n, r; struct timespec tspec; struct fd_map* fm; + int orig_changes; + int apply_changes; int revents; tspec.tv_sec=t; tspec.tv_nsec=0; + orig_changes=h->kq_nchanges; + apply_changes=orig_changes; + do { again: - n=kevent(h->kq_fd, h->kq_changes, h->kq_nchanges, h->kq_array, + n=kevent(h->kq_fd, h->kq_changes, apply_changes, h->kq_array, h->fd_no, &tspec); if (unlikely(n==-1)){ if (errno==EINTR) goto again; /* signal, ignore it */ - else{ + else if (errno==EBADF) { + /* some of the FDs in kq_changes are bad (already closed) + and there is not enough space in kq_array to return all + of them back */ + apply_changes = h->fd_no; + goto again; + }else{ LOG(L_ERR, "ERROR: io_wait_loop_kqueue: kevent:" " %s [%d]\n", strerror(errno), errno); goto error; } } - h->kq_nchanges=0; /* reset changes array */ + /* remove applied changes */ + h->kq_nchanges -= apply_changes; + if (unlikely(apply_changes < orig_changes)) { + orig_changes -= apply_changes; + memmove(&h->kq_changes[0], &h->kq_changes[apply_changes], + sizeof(h->kq_changes[0])*h->kq_nchanges); + apply_changes = orig_changes<h->fd_no ? orig_changes : h->fd_no; + } else { + orig_changes = 0; + apply_changes = 0; + } for (r=0; r<n; r++){ #ifdef EXTRA_DEBUG DBG("DBG: kqueue: event %d/%d: fd=%d, udata=%lx, flags=0x%x\n", @@ -1148,6 +1195,7 @@ again: } } } + } while(unlikely(orig_changes)); error: return n; }

14 years, 5 months

git:kamailio_3.0: io_wait: kqueue: use the entire array during too many errors fallback

by Andrei Pelinescu-Onciul

Module: sip-router Branch: kamailio_3.0 Commit: aed168981e6dae1333afcdaff5976b44c4fac738 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=aed1689… Author: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Date: Thu Jul 8 15:18:52 2010 +0200 io_wait: kqueue: use the entire array during too many errors fallback Minor fix/optimization: if there are too many errors in the changelist and the kevent() call has to be retried, use the entire array (don't rely on the current watched fd number which will be smaller then the array real size, since commit 996826). (only kqueue using systems are affected by this fix: *bsd and darwin) (cherry picked from commit a9cdfc2938ca73d6ba40f5896c6a8930c2e73f85) --- io_wait.h | 105 +++++++++++++++++++++++++++++++------------------------------ 1 files changed, 53 insertions(+), 52 deletions(-) Diff: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commitdiff;h=aed…

14 years, 5 months

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

sr-dev