You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by xiaosuo <gi...@git.apache.org> on 2017/03/10 05:39:21 UTC
[GitHub] thrift pull request #1211: Fix use closed(freed) connections
GitHub user xiaosuo opened a pull request:
https://github.com/apache/thrift/pull/1211
Fix use closed(freed) connections
When failing to add tasks into the thread manager, we close the
corresponding connections, then set the flags of these connections,
which have been already freed.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/xiaosuo/thrift use_after_free
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/thrift/pull/1211.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1211
----
commit b91164a2ca64d465a486ec1e9273c4749fa884a6
Author: Changli Gao <xi...@gmail.com>
Date: 2017-03-10T05:25:43Z
Fix use closed(freed) connections
When failing to add tasks into the thread manager, we close the
corresponding connections, then set the flags of these connections,
which have been already freed.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift issue #1211: Fix use closed(freed) connections
Posted by xiaosuo <gi...@git.apache.org>.
Github user xiaosuo commented on the issue:
https://github.com/apache/thrift/pull/1211
I had tried to find my password, but nothing was received. It seems that Jira doesn't work properly with gmail. Could you help me?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift issue #1211: Fix use closed(freed) connections
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1211
This patch as-is causes a counting error, so if you can amend this one with those changes we can get them all fixed together.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift issue #1211: Fix use closed(freed) connections
Posted by xiaosuo <gi...@git.apache.org>.
Github user xiaosuo commented on the issue:
https://github.com/apache/thrift/pull/1211
@jeking3 Did you mean that we should fix all issues about the number of active processors in this patch instead of a separated one?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift issue #1211: Fix use closed(freed) connections
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1211
If you have the fixes let's get them all in together. The current fix is not complete and causes counting errors, which lead to - I don't know...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift issue #1211: Fix use closed(freed) connections
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1211
THRIFT-4160
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift issue #1211: Fix use closed(freed) connections
Posted by xiaosuo <gi...@git.apache.org>.
Github user xiaosuo commented on the issue:
https://github.com/apache/thrift/pull/1211
@jeking3 you are correct. In fact, we had found this issue, and we should do such things at all the error path when notifying the io threads. I will address this issue in a separated patch.
At the same time, we should setIdle() before calling addTask(). Because the added task maybe expired and destroyed by other threads due to failing to notify IO threads, and the latter setIdle() may corrupt the memory.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift issue #1211: Fix use closed(freed) connections
Posted by xiaosuo <gi...@git.apache.org>.
Github user xiaosuo commented on the issue:
https://github.com/apache/thrift/pull/1211
I think this patch doesn't introduce any inconsistency. And the correctness of the number of active processors deserves another separated patch. We'd better not mix fixes into a huge patch.
BTW: Fixing the number of active processors may depend on my other patches. So I prefer to submit it after all my patches are merged.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift issue #1211: Fix use closed(freed) connections
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1211
@xiaosuo just a reminder, if you have additional fixes (it sounds like you do)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request #1211: Fix use closed(freed) connections
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/thrift/pull/1211
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift issue #1211: Fix use closed(freed) connections
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1211
Please rebase this branch against upstream/master and force push to kick off a new build. I would like to see the CI build pass (or get much closer than it is now) before merging.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---