Fix releasing of requests and responses on client disconnects #969
Conversation
| @@ -1803,6 +1801,14 @@ tfw_http_conn_cli_drop(TfwCliConn *cli_conn) | |||
| list_for_each_entry_safe(req, tmp, seq_queue, msg.seq_list) { | |||
| req->flags |= TFW_HTTP_F_REQ_DROP; | |||
| list_del_init(&req->msg.seq_list); | |||
There was a problem hiding this comment.
It seems we should move list_del_init() from tfw_http_resp_pair_free() to the loop in __tfw_http_resp_fwd() while all other calls of tfw_http_resp_pair_free() are either of two cases of using the function: the call after list_del_init() and full list removal.
| */ | ||
| if (req->resp && (req->resp->flags & TFW_HTTP_F_RESP_READY)) { | ||
| tfw_http_resp_pair_free(req); | ||
| TFW_INC_STAT_BH(serv.msgs_otherr); |
There was a problem hiding this comment.
Why this is a server error? It seems that a server successfully respond to the request and we're here just because the client drops the connection?
There was a problem hiding this comment.
Here is output of performance statistics before the fix:
. . .
Server messages received : 20
Server messages forwarded : 16
Server messages parsing errors : 0
Server messages filtered out : 0
Server messages other errors : 3
. . .
In this stats we see, that one response have disappeared (received != forwarded + p_errors + filtered + o_errors). Other errors is the most suitable counter, which is used for the same situation in other places in code, e.g.
Lines 2209 to 2214 in 43e35e9
tempesta_fw/http_msg.c
Outdated
| @@ -948,6 +948,9 @@ tfw_http_msg_free(TfwHttpMsg *m) | |||
| if (!m) | |||
| return; | |||
|
|
|||
| /* Check that the paired response was destroyed before request. */ | |||
| if (m->conn && (TFW_CONN_TYPE(m->conn) & Conn_Clnt)) | |||
| WARN_ON_ONCE(m->pair); | |||
There was a problem hiding this comment.
Usually we don't write assertions this way, but place the whole condition into the assertion.
See explanation in #959 (comment).