fix: #19 - A Connection could not be closed before it is scheduled

The commit fixes closing a connection at a point when no TfwSession is
yet established.

Changes:

1. tfw_connection_free() now releases a TfwSession and a peer
TfwConnection only if they exist.

2. tfw_connection_send_srv() doesn't close the connection by itself.
Instead, it returns an error code which is propagated up to Synchronous
Sockets, and the connection is closed from ss_do_close().
That is done to avoid a condition where the connection is referenced by
SS after it is closed (and free()'d) in tfw_connection_send_srv().

3. tfw_sess_conn() and tfw_connection_peer() now check for existence of
session and connection objects and return NULL if they don't exist.

4. tfw_http_conn_destruct() frees a HTTP request attached to the
message only if it exists. That is done to avoid another NULL pointer
dereference that occurs because the requests are pipelined now, and
this "current" request may be already pipelined and set to NULL when
the destructor is called.

5. tfw_http_req_process() doesn't free a HTTP request anymore.
It is done from a destructor called by ss_do_close() when
tfw_http_req_process() returns TFW_BLOCK.

6. tfw_session_free() now releases all pipelined HTTP requests stored
in the session's queue.

tempesta_fw/connection.c

@@ -55,15 +55,21 @@ tfw_connection_alloc(int type, void *handler)
 static void
 tfw_connection_free(TfwConnection *c)
 {
-	TfwConnection *peer_conn = tfw_connection_peer(c);
+	TFW_DBG("Free connection: %p\n", c);
 
-	/*
-	 * FIXME do we need to synchronize this?
-	 * If a connection can be processed from different CPUs, then we do.
-	 */
-	peer_conn->sess = NULL;
+	if (c->sess) {
+		/*
+		 * FIXME do we need to synchronize this?
+		 * If a connection can be processed from different CPUs, then we do.
+		 */
+		TfwConnection *peer_conn = tfw_connection_peer(c);
+		if (peer_conn) {
+			TFW_DBG("Detach from peer: %p\n", peer_conn);
+			peer_conn->sess = NULL;
+		}
+		tfw_session_free(c->sess);
+	}
 
-	tfw_session_free(c->sess);
 	kmem_cache_free(conn_cache, c);
 }
 
@@ -134,7 +140,7 @@ tfw_connection_send_cli(TfwSession *sess, TfwMsg *msg)
 	ss_send(sess->cli->sock, &msg->skb_list);
 }
 
-void
+int
 tfw_connection_send_srv(TfwSession *sess, TfwMsg *msg)
 {
 	TfwConnection *srv_conn;
@@ -152,8 +158,7 @@ tfw_connection_send_srv(TfwSession *sess, TfwMsg *msg)
 	if (tfw_session_sched_msg(sess, msg)) {
 		TFW_ERR("Cannot schedule message, msg=%p clnt=%p\n",
 			msg, sess->cli);
-		tfw_connection_close(sess->cli->sock);
-		return;
+		return -1;
 	}
 
 	/* Bind the server connection with the session. */
@@ -167,6 +172,8 @@ tfw_connection_send_srv(TfwSession *sess, TfwMsg *msg)
 	srv_conn->sess = sess;
 
 	ss_send(sess->srv->sock, &msg->skb_list);
+
+	return 0;
 }
 
 /*

tempesta_fw/connection.h

@@ -83,24 +83,31 @@ typedef struct {
 static inline TfwConnection *
 tfw_sess_conn(TfwSession *sess, int type)
 {
-	if (type & Conn_Clnt)
+	if ((type & Conn_Clnt) && sess->cli)
 		return sess->cli->sock->sk_user_data;
-	return sess->srv->sock->sk_user_data;
+	if ((type & Conn_Srv) && sess->srv)
+		return sess->srv->sock->sk_user_data;
+
+	return NULL;
 }
 
 static inline TfwConnection *
 tfw_connection_peer(TfwConnection *c)
 {
+	if (!c->sess)
+		return NULL;
+
 	if (c->type & Conn_Clnt)
 		return tfw_sess_conn(c->sess, Conn_Srv);
+
 	return tfw_sess_conn(c->sess, Conn_Clnt);
 }
 
 /* Connection downcalls. */
 int tfw_connection_new(struct sock *sk, int type, void *handler,
 		       void (*destructor)(struct sock *s));
 void tfw_connection_send_cli(TfwSession *sess, TfwMsg *msg);
-void tfw_connection_send_srv(TfwSession *sess, TfwMsg *msg);
+int tfw_connection_send_srv(TfwSession *sess, TfwMsg *msg);
 
 void tfw_connection_hooks_register(TfwConnHooks *hooks, int type);
 

tempesta_fw/http.c

@@ -57,7 +57,8 @@ tfw_http_conn_init(TfwConnection *conn)
 static void
 tfw_http_conn_destruct(TfwConnection *conn)
 {
-	tfw_http_msg_free((TfwHttpMsg *)conn->msg);
+	if (conn->msg)
+		tfw_http_msg_free((TfwHttpMsg *)conn->msg);
 }
 
 /**
@@ -685,8 +686,11 @@ tfw_http_req_process(TfwConnection *conn, unsigned char *data, size_t len)
 		} else {
 			if (tfw_http_adjust_req(req))
 				goto block;
+
 			/* Send the request to appropriate server. */
-			tfw_connection_send_srv(sess, (TfwMsg *)req);
+			if (tfw_connection_send_srv(sess, (TfwMsg *)req)) {
+				goto block;
+			}
 		}
 
 		if (!req->parser.data_off || req->parser.data_off == len)
@@ -704,7 +708,6 @@ tfw_http_req_process(TfwConnection *conn, unsigned char *data, size_t len)
 
 	return r;
 block:
-	tfw_http_msg_free((TfwHttpMsg *)req);
 	return TFW_BLOCK;
 }
 

tempesta_fw/http_msg.c

@@ -55,6 +55,8 @@ tfw_http_msg_alloc(int type)
 void
 tfw_http_msg_free(TfwHttpMsg *m)
 {
+	TFW_DBG("Free msg: %p", m);
+
 	/*
 	 * FIXME do we need to synchronize this?
 	 * If a connection can be processed from different CPUs, then we do.

tempesta_fw/session.c

@@ -19,6 +19,10 @@
  * this program; if not, write to the Free Software Foundation, Inc., 59
  * Temple Place - Suite 330, Boston, MA 02111-1307, USA.
  */
+
+#include <linux/list.h>
+
+#include "http_msg.h"
 #include "log.h"
 #include "sched.h"
 #include "session.h"
@@ -56,6 +60,16 @@ tfw_session_create(TfwClient *cli)
 void
 tfw_session_free(TfwSession *s)
 {
+	TfwHttpReq *req, *tmp;
+
+	TFW_DBG("Free session: %p\n", s);
+
+	/* Release all pipelined HTTP requests. */
+	list_for_each_entry_safe(req, tmp, &s->req_list, list) {
+		list_del(&req->list);
+		tfw_http_msg_free((TfwHttpMsg *)req);
+	}
+
 	kmem_cache_free(sess_cache, s);
 }