You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by oknet <gi...@git.apache.org> on 2015/12/07 15:51:04 UTC
[GitHub] trafficserver pull request: remove alloc_cache from NetAccept
GitHub user oknet opened a pull request:
https://github.com/apache/trafficserver/pull/366
remove alloc_cache from NetAccept
remove alloc_cache from NetAccept & net_accept() to avoid mem leak on ~NetAccept().
add a condition check for vc==NULL follow allocate_vc().
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/oknet/trafficserver master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafficserver/pull/366.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 #366
----
commit 37fd8eddf4008e0335b39995d669bab62e5719c7
Author: Oknet <xu...@gmail.com>
Date: 2015-12-07T14:13:42Z
remove alloc_cache from class NetAccept definition
commit 736fd108441ff0743e69d64c1b146e2c65f359d9
Author: Oknet <xu...@gmail.com>
Date: 2015-12-07T14:42:27Z
remove alloc_cache from net_accept()
remove alloc_cache from net_accept() to avoid mem leak on ~NetAccept()
commit 5243fa51a56cb51d84bd94db9981c1271f70f231
Author: Oknet <xu...@gmail.com>
Date: 2015-12-07T14:47:11Z
fix vc->con to con
----
---
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] trafficserver pull request: remove alloc_cache from NetAccept
Posted by oknet <gi...@git.apache.org>.
Github user oknet commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/366#discussion_r46964787
--- Diff: iocore/net/UnixNetAccept.cc ---
@@ -112,9 +106,17 @@ net_accept(NetAccept *na, void *ep, bool blockable)
count = res;
goto Ldone;
}
+
+ vc = (UnixNetVConnection *)na->getNetProcessor()->allocate_vc(e->ethread);
+ if (!vc) {
+ con.close();
+ goto Ldone;
+ }
+
count++;
- na->alloc_cache = NULL;
-
+ NET_SUM_GLOBAL_DYN_STAT(net_connections_currently_open_stat, 1);
+ vc->id = net_next_connection_number();
+ vc->con = con;
--- End diff --
Connection::move() not existed, I'm set the con.fd to NO_FD to avoid con.fd closed by ~Connection()
---
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] trafficserver pull request: remove alloc_cache from NetAccept
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on the pull request:
https://github.com/apache/trafficserver/pull/366#issuecomment-162560602
A few questions -
1. Why is it the destruction of a `NetAccept` a concern - I thought that only happened during process shutdown. Is there a case where it happens during process execution? Or this a general cleanup?
2. Is `net_accept` actually called? I think it might be called if a plugin calls `TSNetAccept` but I would be interested in confirmation of that.
3. Why not have the destructor of `NetAccept`
---
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] trafficserver pull request: TS-4056: remove alloc_cache from NetAc...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/trafficserver/pull/366
---
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] trafficserver pull request: remove alloc_cache from NetAccept
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/366#discussion_r46833574
--- Diff: iocore/net/UnixNetAccept.cc ---
@@ -112,9 +106,17 @@ net_accept(NetAccept *na, void *ep, bool blockable)
count = res;
goto Ldone;
}
+
+ vc = (UnixNetVConnection *)na->getNetProcessor()->allocate_vc(e->ethread);
+ if (!vc) {
+ con.close();
+ goto Ldone;
+ }
+
count++;
- na->alloc_cache = NULL;
-
+ NET_SUM_GLOBAL_DYN_STAT(net_connections_currently_open_stat, 1);
+ vc->id = net_next_connection_number();
+ vc->con = con;
--- End diff --
I think `vc->con.move(con)` would be better here.
---
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] trafficserver pull request: remove alloc_cache from NetAccept
Posted by oknet <gi...@git.apache.org>.
Github user oknet commented on the pull request:
https://github.com/apache/trafficserver/pull/366#issuecomment-162803942
Answers：
1. to implement an one shot accept (like a ftp pasv mode)
2. accept_fn = net_accept, accept_fn is called from NetAccept
3. the alloc_cache point to a vc, the vc may be a UnixNetVConnection, sslNetVConnection, or others ...
should dealloc it by netAllocate.free(), sslNetAllocate.free() ...
It is hardly to get the vc's type inside the destructor of NetAccept.
---
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.
---