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:sck@;lmt.lv
Latvian Mobile Phone Company http://www.lmt.lv
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);
>
