You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by myraid <gi...@git.apache.org> on 2016/01/29 01:16:30 UTC
[GitHub] trafficserver pull request: [TS-4135] fix redirect coredump
GitHub user myraid opened a pull request:
https://github.com/apache/trafficserver/pull/442
[TS-4135] fix redirect coredump
@bgaff @shukitchan @sudheerv can you review.
Removing TSMlocRelease is harmless as it s a no-op. Until we come up with a better way of cleanup lets not release the mloc for request and response headers.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/myraid/trafficserver TS-4135
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafficserver/pull/442.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 #442
----
commit b4b5204029feff8aa451c1105e45bd28e67c5437
Author: Sandeep Davu <sd...@sniffcliff.corp.ne1.yahoo.com>
Date: 2016-01-29T00:13:00Z
[TS-4135] fix redirect coredump
----
---
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-4135] fix redirect coredump
Posted by sudheerv <gi...@git.apache.org>.
Github user sudheerv commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/442#discussion_r51277409
--- Diff: lib/atscppapi/src/Transaction.cc ---
@@ -433,14 +411,12 @@ class initializeHandles
initializeHandles(GetterFunction getter) : getter_(getter) {}
bool operator()(TSHttpTxn txn, TSMBuffer &hdr_buf, TSMLoc &hdr_loc, const char *handles_name)
{
- if (!hdr_buf && !hdr_loc) {
- if (getter_(txn, &hdr_buf, &hdr_loc) == TS_SUCCESS) {
- return true;
- } else {
- LOG_ERROR("Could not get %s", handles_name);
- }
+ hdr_buf = NULL;
+ hdr_loc = NULL;
+ if (getter_(txn, &hdr_buf, &hdr_loc) == TS_SUCCESS) {
+ return true;
} else {
- LOG_ERROR("%s already initialized", handles_name);
+ LOG_ERROR("Could not get %s", handles_name);
--- End diff --
The handles inside the Request and Response objects need to be reset here as well, otherwise, plugins may reference invalid handles via those 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 pull request: [TS-4135] fix redirect coredump
Posted by shukitchan <gi...@git.apache.org>.
Github user shukitchan commented on the pull request:
https://github.com/apache/trafficserver/pull/442#issuecomment-179556276
I think we can close this now. the changes are merged. here are the commits
17cc1561b25bb4f1be9834d9f7e94dcdeef868bf
919de4cc8daa5ee739aa7993b9de59dd72530ee0
74d8bb48381393075ae82a7deedb384937b1c285
---
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-4135] fix redirect coredump
Posted by sudheerv <gi...@git.apache.org>.
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/442#issuecomment-177043261
:+1:
---
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-4135] fix redirect coredump
Posted by sudheerv <gi...@git.apache.org>.
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/442#issuecomment-177007742
Would it be better to move the reset() into the getter() inside the operator()?
---
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-4135] fix redirect coredump
Posted by bgaff <gi...@git.apache.org>.
Github user bgaff commented on the pull request:
https://github.com/apache/trafficserver/pull/442#issuecomment-177737313
I'm okay with this, but I do have a comment on #443
---
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-4135] fix redirect coredump
Posted by sudheerv <gi...@git.apache.org>.
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/442#issuecomment-176504036
please review https://github.com/apache/trafficserver/pull/443
---
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-4135] fix redirect coredump
Posted by myraid <gi...@git.apache.org>.
Github user myraid commented on the pull request:
https://github.com/apache/trafficserver/pull/442#issuecomment-179556404
Closing the PR.
---
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-4135] fix redirect coredump
Posted by myraid <gi...@git.apache.org>.
Github user myraid commented on the pull request:
https://github.com/apache/trafficserver/pull/442#issuecomment-177009174
we would not know which object to reset.
---
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-4135] fix redirect coredump
Posted by sudheerv <gi...@git.apache.org>.
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/442#issuecomment-176500044
This patch does not solve the issue completely (for example, the issues raised in https://issues.apache.org/jira/browse/TS-4160). You will also need to reset the handles on every hook to make sure that invalid hooks are assigned. I'm creating a PR shortly which will supplement this.
---
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-4135] fix redirect coredump
Posted by sudheerv <gi...@git.apache.org>.
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/442#issuecomment-176516828
:+1: but, need to go along with https://github.com/apache/trafficserver/pull/443
---
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-4135] fix redirect coredump
Posted by myraid <gi...@git.apache.org>.
Github user myraid commented on the pull request:
https://github.com/apache/trafficserver/pull/442#issuecomment-176964528
updated the request/response objects to reset the handles to null n failure.
@sudheerv can you review.
---
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-4135] fix redirect coredump
Posted by myraid <gi...@git.apache.org>.
Github user myraid closed the pull request at:
https://github.com/apache/trafficserver/pull/442
---
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.
---