You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by am...@apache.org on 2015/05/08 17:58:16 UTC
trafficserver git commit: TS-3370: header_rewrite plugin uses dead
continuation. This closes #173
Repository: trafficserver
Updated Branches:
refs/heads/master 08a0efed3 -> be0b7311d
TS-3370: header_rewrite plugin uses dead continuation.
This closes #173
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/be0b7311
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/be0b7311
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/be0b7311
Branch: refs/heads/master
Commit: be0b7311dfd5f7645726d83ab967e0e0c527f4ef
Parents: 08a0efe
Author: Alan M. Carroll <am...@apache.org>
Authored: Wed Feb 25 16:27:56 2015 -0600
Committer: Alan M. Carroll <am...@apache.org>
Committed: Fri May 8 10:57:30 2015 -0500
----------------------------------------------------------------------
CHANGES | 2 +
plugins/header_rewrite/header_rewrite.cc | 57 ++++++++++++++++++++-------
2 files changed, 44 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/be0b7311/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index 3a51f88..206cd04 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,8 @@
-*- coding: utf-8 -*-
Changes with Apache Traffic Server 6.0.0
+ *) [TS-3370] header_rewrite plugin uses dead continuation.
+
*) [TS-3588]: Fix continuation leak in background_fetch in remap mode
*) [TS-3029]: Fix spdy related seg fault in production, due to resetting
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/be0b7311/plugins/header_rewrite/header_rewrite.cc
----------------------------------------------------------------------
diff --git a/plugins/header_rewrite/header_rewrite.cc b/plugins/header_rewrite/header_rewrite.cc
index ca85a24..95b6a1a 100644
--- a/plugins/header_rewrite/header_rewrite.cc
+++ b/plugins/header_rewrite/header_rewrite.cc
@@ -21,6 +21,8 @@
#include "ts/ts.h"
#include "ts/remap.h"
+#include "ts/ink_atomic.h"
+
#include "parser.h"
#include "ruleset.h"
#include "resources.h"
@@ -32,13 +34,12 @@ const char PLUGIN_NAME_DBG[] = "dbg_header_rewrite";
// Forward declaration for the main continuation.
static int cont_rewrite_headers(TSCont, TSEvent, void *);
-
// Simple wrapper around a configuration file / set. This is useful such that
// we can reuse most of the code for both global and per-remap rule sets.
class RulesConfig
{
public:
- RulesConfig()
+ RulesConfig() : _ref_count(0)
{
memset(_rules, 0, sizeof(_rules));
memset(_resids, 0, sizeof(_resids));
@@ -47,15 +48,6 @@ public:
TSContDataSet(_cont, static_cast<void *>(this));
}
- ~RulesConfig()
- {
- for (int i = TS_HTTP_READ_REQUEST_HDR_HOOK; i < TS_HTTP_LAST_HOOK; ++i) {
- delete _rules[i];
- }
-
- TSContDestroy(_cont);
- }
-
TSCont
continuation() const
{
@@ -75,10 +67,31 @@ public:
bool parse_config(const std::string fname, TSHttpHookID default_hook);
+ void
+ hold()
+ {
+ ink_atomic_increment(&_ref_count, 1);
+ }
+ void
+ release()
+ {
+ if (1 >= ink_atomic_decrement(&_ref_count, 1))
+ delete this;
+ }
+
private:
+ ~RulesConfig()
+ {
+ for (int i = TS_HTTP_READ_REQUEST_HDR_HOOK; i < TS_HTTP_LAST_HOOK; ++i) {
+ delete _rules[i];
+ }
+ TSContDestroy(_cont);
+ }
+
bool add_rule(RuleSet *rule);
TSCont _cont;
+ volatile int _ref_count;
RuleSet *_rules[TS_HTTP_LAST_HOOK + 1];
ResourceIDs _resids[TS_HTTP_LAST_HOOK + 1];
};
@@ -236,6 +249,9 @@ cont_rewrite_headers(TSCont contp, TSEvent event, void *edata)
case TS_EVENT_HTTP_SEND_RESPONSE_HDR:
hook = TS_HTTP_SEND_RESPONSE_HDR_HOOK;
break;
+ case TS_EVENT_HTTP_TXN_CLOSE:
+ conf->release();
+ break;
default:
TSError("%s: unknown event for this plugin", PLUGIN_NAME);
TSDebug(PLUGIN_NAME, "unknown event for this plugin");
@@ -288,6 +304,8 @@ TSPluginInit(int argc, const char *argv[])
RulesConfig *conf = new RulesConfig;
bool got_config = false;
+ conf->hold();
+
for (int i = 1; i < argc; ++i) {
// Parse the config file(s). Note that multiple config files are
// just appended to the configurations.
@@ -313,7 +331,7 @@ TSPluginInit(int argc, const char *argv[])
} else {
// Didn't get anything, nuke it.
TSError("%s: failed to parse configuration file", PLUGIN_NAME);
- delete conf;
+ conf->release();
}
}
@@ -357,6 +375,8 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf ATS_UNUSE
RulesConfig *conf = new RulesConfig;
+ conf->hold();
+
for (int i = 2; i < argc; ++i) {
TSDebug(PLUGIN_NAME, "Loading remap configuration file %s", argv[i]);
if (!conf->parse_config(argv[i], TS_REMAP_PSEUDO_HOOK)) {
@@ -384,9 +404,7 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf ATS_UNUSE
void
TSRemapDeleteInstance(void *ih)
{
- RulesConfig *conf = static_cast<RulesConfig *>(ih);
-
- delete conf;
+ static_cast<RulesConfig *>(ih)->release();
}
@@ -396,6 +414,8 @@ TSRemapDeleteInstance(void *ih)
TSRemapStatus
TSRemapDoRemap(void *ih, TSHttpTxn rh, TSRemapRequestInfo *rri)
{
+ bool hooked_p = false;
+
// Make sure things are properly setup (this should never happen)
if (NULL == ih) {
TSDebug(PLUGIN_NAME, "No Rules configured, falling back to default");
@@ -408,11 +428,18 @@ TSRemapDoRemap(void *ih, TSHttpTxn rh, TSRemapRequestInfo *rri)
// Go through all hooks we support, and setup the txn hook(s) as necessary
for (int i = TS_HTTP_READ_REQUEST_HDR_HOOK; i < TS_HTTP_LAST_HOOK; ++i) {
if (conf->rule(i)) {
+ hooked_p = true;
TSHttpTxnHookAdd(rh, static_cast<TSHttpHookID>(i), conf->continuation());
TSDebug(PLUGIN_NAME, "Added remapped TXN hook=%s", TSHttpHookNameLookup((TSHttpHookID)i));
}
}
+ // Two assumptions - configuration never uses this hook nor uses TS_HTTP_SSN_CLOSE_HOOK.
+ if (hooked_p) {
+ conf->hold(); // mark as in use.
+ TSHttpTxnHookAdd(rh, TS_HTTP_TXN_CLOSE_HOOK, conf->continuation()); // clean up after.
+ }
+
// Now handle the remap specific rules for the "remap hook" (which is not a real hook).
// This is sufficiently differen than the normal cont_rewrite_headers() callback, and
// we can't (shouldn't) schedule this as a TXN hook.