Re: Kannel & blocking connect again.
Vjacheslav Chekushin <[email protected]>
| Newsgroups | gmane.comp.mobile.kannel.devel |
|---|---|
| Organization | LMT |
| Message-ID | <[email protected]> |
Hi, I think, this patch is clean enough. I run kannel with this patch in commercial enviroment about a week and have no problem. Still remain problem with host resolving, but there is no portable solution to avoid mutex. I suggest to use getaddrinfo for all systems which supports it. Stipe Tolj wrote: >>Scenario 1 (how to reproduce) >>As I said it is very easy to reproduce if you have access to any web server >>with public address (you can test it with private web server if wapgw has >>access to it, of course). >>Fisrt of all configure some port to DROP (not REJECT!) incoming SYN packets >>(or all packets). >>Then take 2 phones or phone&fakewap. Make request to configured port from first >>device. Kannel will block for 1-2 minutes. If you immediatly send request from >>another device you will get timeout error. >> > > Hmmm, ok... > > >>Scenario 2 (kannel process description) >>1. http library get request from queue pending_requests >>2. make connect to host >>3. send request (nonblock realized by qwpoll and fdset) >>4. create callback for reply >>5. go to step 1. >> >>Take step 2: >>2.1 connect() send SYN to host >>2.2 Wait for ACK (blocking till some minutes if no ACK or RST) >>2.3 And handshaking and return connected socket or error. >> >>So if we block at 2.2 then all requests in pending_request queue will be lost >>due timeout for session machine. >> > > sounds reasonable, yes. > > >>Is this patch was tested by someone? With this patch even working >>request doesn't work: >> > > :(((, no I didn't test. > > >>BTW, I have patch that solve problem of blocking connect, but it is not >>so complex as introduced by Netikos - it doesn't handle maximum number of >>open connections to the server and don't use getaddrinfo for resolving. >> > > so you should have posted that already to the list to be commited to > cvs :))) please forward it to the list as a clean diff -u against the > current cvs tree. > > Stipe > > [email protected] > ------------------------------------------------------------------- > Wapme Systems AG > > -- Vjacheslav Chekushin mailto:[email protected] Latvian Mobile Phone Company http://www.lmt.lv
nonblock.patch
(text/plain, 12.1 KB)
Index: gwlib/conn.c
===================================================================
RCS file: /home/cvs/gateway/gwlib/conn.c,v
retrieving revision 1.55
diff -r1.55 conn.c
64a65,67
> /* socket state */
> enum {yes,no} connected;
>
462a466,510
> Connection *conn_open_tcp_nb(Octstr *host, int port, Octstr *our_host)
> {
> return conn_open_tcp_nb_with_port(host, port, 0, our_host);
> }
>
> Connection *conn_open_tcp_nb_with_port(Octstr *host, int port, int our_port,
> Octstr *our_host)
> {
> int sockfd;
> int done = -1;
> Connection *c;
>
> sockfd = tcpip_connect_nb_to_server_with_port(octstr_get_cstr(host), port,
> our_port, our_host == NULL ?
> NULL : octstr_get_cstr(our_host), &done);
> if (sockfd < 0)
> return NULL;
> c = conn_wrap_fd(sockfd, 0);
> if (done != 0) {
> c->connected = no;
> }
> return c;
> }
>
> int conn_is_connected(Connection *conn)
> {
> if (conn->connected == yes) return 0;
> return -1;
> }
>
> int conn_get_connect_result(Connection *conn)
> {
> int err,len;
> len = sizeof(len);
> if (getsockopt(conn->fd, SOL_SOCKET, SO_ERROR, &err, &len) < 0) {
> return -1;
> }
>
> if (err) {
> return -1;
> }
>
> conn->connected = yes;
> return 0;
> }
495a544
> conn->connected = yes;
748a798,804
> /* Get result of nonblocking connect, before any reads and writes
> * we must check result (it must be handled in initial callback) */
> if (conn->connected == no) {
> conn->callback(conn, conn->callback_data);
> return;
> }
>
796a853,854
> /* For nonconnected socket we must lesten both directions */
> if (conn->connected == yes) {
800a859,862
> } else {
> events |= POLLIN;
> events |= POLLOUT;
> }
Index: gwlib/conn.h
===================================================================
RCS file: /home/cvs/gateway/gwlib/conn.h,v
retrieving revision 1.23
diff -r1.23 conn.h
77a78,94
> /* Open a TCP/IP connection to the given host and port. Return NULL in case of
> * error. Overwise return new Connection. */
> Connection *conn_open_tcp_nb(Octstr *host, int port, Octstr *our_host);
>
> /* As above, but binds our end to 'our_port'. If 'our_port' is 0, uses
> * any port like conn_open_tcp. */
> Connection *conn_open_tcp_nb_with_port(Octstr *host, int port, int our_port,
> Octstr *our_host);
>
> /* Returns 0 if socket is connected, -1 overwise */
> int conn_is_connected(Connection *conn);
>
> /* If socket is in the 'connecting' state, it must be listen by poller.
> * After poller returns, connection must be checked for connection
> * procedure's result. Return 0 if connection done successfully */
> int conn_get_connect_result(Connection *conn);
>
Index: gwlib/http.c
===================================================================
RCS file: /home/cvs/gateway/gwlib/http.c,v
retrieving revision 1.175
diff -r1.175 http.c
553d552
<
564a564
> connecting,
584a585,586
> static int send_request(HTTPServer *trans);
>
701c703
< conn = conn_open_tcp(host, port, our_host);
---
> conn = conn_open_tcp_nb(host, port, our_host);
854c856,858
<
---
> int rc;
> char buf[128];
>
863a868,904
> case connecting:
> debug("gwlib.http", 0, "Get info about connecting socket");
> if (conn_get_connect_result(trans->conn) != 0) {
> debug("gwlib.http", 0, "Socket not connected");
> conn_unregister(conn);
> goto error;
> }
>
> if (trans->method == HTTP_METHOD_POST) {
> /*
> * Add a Content-Length header. Override an existing one, if
> * necessary. We must have an accurate one in order to use the
> * connection for more than a single request.
> */
> http_header_remove_all(trans->request_headers, "Content-Length");
> sprintf(buf, "%ld", octstr_len(trans->request_body));
> http_header_add(trans->request_headers, "Content-Length", buf);
> }
> /*
> * ok, this has to be an GET or HEAD request method then,
> * if it contains a body, then this is not HTTP conform, so at
> * least warn the user
> */
> else if (trans->request_body != NULL) {
> warning(0, "HTTP: GET or HEAD method request contains body:");
> octstr_dump(trans->request_body, 0);
> }
>
> if ((rc = send_request(trans)) == 0) {
> trans->state = reading_status;
> conn_register(trans->conn, client_fdset, handle_transaction,
> trans);
> } else {
> list_produce(trans->caller, trans);
> }
> break;
>
1123,1129c1164
<
< /*
< * Build and send the HTTP request. Return socket from which the
< * response can be read or -1 for error.
< */
<
< static Connection *send_request(HTTPServer *trans)
---
> static Connection *get_connection(HTTPServer *trans)
1131c1166
< Octstr *path, *request;
---
> Octstr *path;
1136,1137d1170
< path = NULL;
< request = NULL;
1138a1172
> path = NULL;
1144,1146d1177
< if(trans->request_headers == NULL)
< trans->request_headers = http_create_empty_headers();
<
1150,1152c1181,1182
< if (trans->username != NULL)
< http_add_basic_auth(trans->request_headers, trans->username,
< trans->password);
---
>
>
1154,1157d1183
< proxy_add_authentication(trans->request_headers);
< request = build_request(http_method2name(trans->method),
< trans->url, trans->host, trans->port,
< trans->request_headers, trans->request_body);
1161,1163d1186
< request = build_request(http_method2name(trans->method), path, trans->host,
< trans->port, trans->request_headers,
< trans->request_body);
1170,1171c1193
< if (trans->ssl)
< conn = conn_open_ssl(host, port, trans->certkeyfile, our_host);
---
> if (trans->ssl) conn = conn_open_ssl(host, port, trans->certkeyfile, our_host);
1174c1196
< conn = conn_open_tcp(host, port, our_host);
---
> conn = conn_open_tcp_nb(host, port, our_host);
1178c1200,1201
< conn = conn_pool_get(host, port, trans->ssl, trans->certkeyfile, our_host);
---
> conn = conn_pool_get(host, port, trans->ssl, trans->certkeyfile,
> our_host);
1181a1205,1248
> octstr_destroy(path);
>
> return conn;
>
> error:
> conn_destroy(conn);
> octstr_destroy(path);
> error(0, "Couldn't send request to <%s>", octstr_get_cstr(trans->url));
> return NULL;
> }
>
> /*
> * Build and send the HTTP request. Return socket from which the
> * response can be read or -1 for error.
> */
>
> static int send_request(HTTPServer *trans)
> {
> Octstr *path, *request;
>
> path = NULL;
> request = NULL;
>
> if (parse_url(trans->url, &trans->host, &trans->port, &path, &trans->ssl,
> &trans->username, &trans->password) == -1)
> goto error;
>
> if (trans->username != NULL)
> http_add_basic_auth(trans->request_headers, trans->username,
> trans->password);
>
> if (proxy_used_for_host(trans->host)) {
> proxy_add_authentication(trans->request_headers);
> request = build_request(http_method2name(trans->method),
> trans->url, trans->host, trans->port,
> trans->request_headers,
> trans->request_body);
> } else {
> request = build_request(http_method2name(trans->method),path,
> trans->host, trans->port,
> trans->request_headers,
> trans->request_body);
> }
>
1184c1251
< if (conn_write(conn, request) == -1)
---
> if (conn_write(trans->conn, request) == -1)
1190c1257
< return conn;
---
> return 0;
1192,1193c1259,1261
< error:
< conn_destroy(conn);
---
> error:
> conn_destroy(trans->conn);
> trans->conn = NULL;
1197c1265
< return NULL;
---
> return -1;
1200d1267
<
1209a1277
> int rc;
1217a1286,1293
> trans->conn = get_connection(trans);
>
> if (trans->conn == NULL)
> list_produce(trans->caller, trans);
> else {
> if (conn_is_connected(trans->conn) == 0) {
> debug("gwlib.http", 0, "Socket connected at once");
>
1237,1241c1313
<
< trans->conn = send_request(trans);
< if (trans->conn == NULL)
< list_produce(trans->caller, trans);
< else {
---
> if ((rc = send_request(trans)) == 0) {
1242a1315,1323
> conn_register(trans->conn, client_fdset, handle_transaction,
> trans);
> } else {
> list_produce(trans->caller, trans);
> }
>
> } else { /* Socket not connected, wait for connection */
> debug("gwlib.http", 0, "Socket connecting");
> trans->state = connecting;
1244a1326,1327
>
> }
Index: gwlib/socket.c
===================================================================
RCS file: /home/cvs/gateway/gwlib/socket.c,v
retrieving revision 1.52
diff -r1.52 socket.c
162a163,248
> int tcpip_connect_nb_to_server(char *hostname, int port, const char *interface_name, int *done)
> {
> return tcpip_connect_nb_to_server_with_port(hostname, port, 0, interface_name, done);
> }
>
> int tcpip_connect_nb_to_server_with_port(char *hostname, int port, int our_port, const char *interface_name, int *done)
> {
> struct sockaddr_in addr;
> struct sockaddr_in o_addr;
> struct hostent hostinfo;
> struct hostent o_hostinfo;
> int s;
> int flags,rc;
>
> *done = 1;
>
> s = socket(PF_INET, SOCK_STREAM, 0);
> if (s == -1) {
> error(errno, "Couldn't create new socket.");
> goto error;
> }
>
> if (gw_gethostbyname(&hostinfo, hostname) == -1) {
> error(errno, "gethostbyname failed");
> goto error;
> }
>
> addr = empty_sockaddr_in;
> addr.sin_family = AF_INET;
> addr.sin_port = htons(port);
> addr.sin_addr = *(struct in_addr *) hostinfo.h_addr;
>
> if (our_port > 0 || (interface_name != NULL && strcmp(interface_name, "*") != 0)) {
> int reuse;
>
> o_addr = empty_sockaddr_in;
> o_addr.sin_family = AF_INET;
> o_addr.sin_port = htons(our_port);
> if (interface_name == NULL || strcmp(interface_name, "*") == 0)
> o_addr.sin_addr.s_addr = htonl(INADDR_ANY);
> else {
> if (gw_gethostbyname(&o_hostinfo, interface_name) == -1) {
> error(errno, "gethostbyname failed");
> goto error;
> }
> o_addr.sin_addr = *(struct in_addr *) o_hostinfo.h_addr;
> }
>
> reuse = 1;
> if (setsockopt(s, SOL_SOCKET, SO_REUSEADDR, (char *) &reuse,
> sizeof(reuse)) == -1) {
> error(errno, "setsockopt failed before bind");
> goto error;
> }
> if (bind(s, (struct sockaddr *) &o_addr, sizeof(o_addr)) == -1) {
> error(errno, "bind to local port %d failed", our_port);
> goto error;
> }
> }
>
> flags = fcntl(s, F_GETFL, 0);
> fcntl(s, F_SETFL, flags | O_NONBLOCK);
>
> if ((rc = connect(s, (struct sockaddr *) &addr, sizeof(addr))) < 0) {
> if (errno != EINPROGRESS) {
> error(errno, "nonblocking connect failed");
> goto error;
> }
> }
>
> /* May be connected immediatly
> * (if we connecting to localhost for example) */
> if (rc == 0) {
> *done = 0;
> }
>
> return s;
>
> error:
> error(0, "error connecting to server `%s' at port `%d'",
> hostname, port);
> if (s >= 0)
> close(s);
> return -1;
> }
>
Index: gwlib/socket.h
===================================================================
RCS file: /home/cvs/gateway/gwlib/socket.h,v
retrieving revision 1.21
diff -r1.21 socket.h
40a41,49
> /* Open a client socket in nonblocking mode, done is 0 if socket
> connected immediatly, overwise done is 1 */
> int tcpip_connect_nb_to_server(char *hostname, int port, const char *interface_name,
> int *done);
>
> /* As above, but binds our end to 'our_port' */
> int tcpip_connect_nb_to_server_with_port(char *hostname, int port, int our_port,
> const char *interface_name, int *done);
>