Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the connection destruct procedure #412

Merged
merged 2 commits into from
Feb 9, 2016
Merged

Fix the connection destruct procedure #412

merged 2 commits into from
Feb 9, 2016

Conversation

keshonok
Copy link
Contributor

@keshonok keshonok commented Feb 2, 2016

No description provided.

if (tfw_http_adjust_resp(resp, req))
goto resp_err;
if (tfw_cli_conn_send(req->conn, (TfwMsg *)resp, true))
goto resp_err;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems there is no sense to try to send client an error message if we couldn't send them normal message. When error message can be sent if normal message can't?

The previous fix has a subtle bug. Suppose a server has transmitted
a part of a response (the result of parsing is TFW_POSTPONE) and died.
tfw_sock_srv_do_failover() is called in that case, and @conn->refcnt
is greater that one at that time. @conn->refcnt is decremented in
tfw_sock_srv_do_failover(), but there's still the reference from the
incomplete response, so the connection destruction procesure does not
proceed to the final connection release stage. No data is coming from
the server anymore, so the response stays in the postponed state.
Therefore it's never freed and it never releases that extra reference
to the connection to allow the connection destruction procedure to
proceed to the final stage. The most visible consequence of that is
that the connection to the server is never restored.

The fix splits the connection destruction procedure in two parts.
One is executed at the time the connection is dropped, and the other
is executed when the connection is finally released after there are
no more users of the connection.
@krizhanovsky
Copy link
Contributor

Good to merge

keshonok added a commit that referenced this pull request Feb 9, 2016
Fix the connection destruct procedure
@keshonok keshonok merged commit 01ae084 into master Feb 9, 2016
@keshonok keshonok deleted the ab-refix-destruct branch February 9, 2016 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants