You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficserver.apache.org by bryancall <gi...@git.apache.org> on 2016/10/11 19:59:07 UTC
[GitHub] trafficserver pull request #1095: TS-4956: Memory leaks in hostdb test
GitHub user bryancall opened a pull request:
https://github.com/apache/trafficserver/pull/1095
TS-4956: Memory leaks in hostdb test
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/bryancall/trafficserver TS-4956
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafficserver/pull/1095.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 #1095
----
commit b818e5dda8e9f538e0fc27b9430c8a25aed6cd9a
Author: Bryan Call <bc...@apache.org>
Date: 2016-10-11T19:58:01Z
TS-4956: Memory leaks in hostdb test
----
---
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 #1095: TS-4956: Memory leaks in hostdb test
Posted by bryancall <gi...@git.apache.org>.
Github user bryancall closed the pull request at:
https://github.com/apache/trafficserver/pull/1095
---
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 #1095: TS-4956: Memory leaks in hostdb test
Posted by jacksontj <gi...@git.apache.org>.
Github user jacksontj commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1095#discussion_r82878003
--- Diff: iocore/hostdb/test_RefCountCache.cc ---
@@ -135,8 +135,6 @@ testRefcounting()
cache->put(1, to_delete);
ret |= to_delete->refcount() != 1;
cache->erase(1);
- ret |= to_delete->refcount() != 0;
--- End diff --
Since the memory is actually getting freed we should figure out a way to test that it was in fact deleted. An alternative is maybe to have it update a global map of key -> bool for things that where deleted? then we could simply do a get and ensure its not in the map. (something to replace the test instead of dropping it. Or maybe a global atomic for number of active ExampleStruct objects
---
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 issue #1095: TS-4956: Memory leaks in hostdb test
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1095
Linux build *failed*! See https://ci.trafficserver.apache.org/job/Github-Linux/868/ for details.
---
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 issue #1095: TS-4956: Memory leaks in hostdb test
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1095
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/860/ for details.
---
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 issue #1095: TS-4956: Memory leaks in hostdb test
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1095
Linux build *failed*! See https://ci.trafficserver.apache.org/job/Github-Linux/997/ for details.
---
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 issue #1095: TS-4956: Memory leaks in hostdb test
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1095
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/976/ for details.
---
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 #1095: TS-4956: Memory leaks in hostdb test
Posted by jacksontj <gi...@git.apache.org>.
Github user jacksontj commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1095#discussion_r82878134
--- Diff: iocore/hostdb/test_RefCountCache.cc ---
@@ -166,6 +164,8 @@ testRefcounting()
ret |= tmpAfter.get()->idx != 1;
printf("ret=%d ref=%d\n", ret, tmp->refcount());
+ delete cache;
--- End diff --
we probably want to verify somehow that the destructor happened
---
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 issue #1095: TS-4956: Memory leaks in hostdb test
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1095
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/968/ for details.
---
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 #1095: TS-4956: Memory leaks in hostdb test
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1095#discussion_r83048541
--- Diff: iocore/hostdb/test_RefCountCache.cc ---
@@ -264,7 +272,20 @@ main()
// printf("Sync return: %d\n", cache->sync_all());
printf("TestRun: %d\n", ret);
- exit(ret);
+
+ delete cache;
+
+ return ret;
+}
+
+int main() {
+ int ret = test();
+
+ for (const auto item: ExampleStruct::items_freed) {
+ printf("really freeing: %p\n", item);
+ ::free(item);
--- End diff --
You still ought to call the destructor when you free.
---
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 #1095: TS-4956: Memory leaks in hostdb test
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1095#discussion_r83048638
--- Diff: iocore/hostdb/test_RefCountCache.cc ---
@@ -264,7 +272,20 @@ main()
// printf("Sync return: %d\n", cache->sync_all());
printf("TestRun: %d\n", ret);
- exit(ret);
+
+ delete cache;
+
+ return ret;
+}
+
+int main() {
+ int ret = test();
+
+ for (const auto item: ExampleStruct::items_freed) {
+ printf("really freeing: %p\n", item);
+ ::free(item);
+ }
+ ExampleStruct::items_freed.clear();
--- End diff --
Since this is a test, how about verifying that all the expected items were 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] trafficserver pull request #1095: TS-4956: Memory leaks in hostdb test
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1095#discussion_r82898650
--- Diff: iocore/hostdb/test_RefCountCache.cc ---
@@ -48,11 +48,11 @@ class ExampleStruct : public RefCountObj
return new (malloc(sizeof(ExampleStruct) + size)) ExampleStruct();
}
- // To mark it as "deleted" (so its easy to check) we'll just mark the idx as -1
+ // Really free the memory, we can use asan leak detection to verify it was freed
void
free()
{
- this->idx = -1;
+ ::free(this);
--- End diff --
Need to call the destructor 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.
---