From 3c3c3f1b039ef36a819034085f51b336eba3b013 Mon Sep 17 00:00:00 2001 From: Sergey Linev Date: Tue, 9 Jun 2026 17:14:26 +0200 Subject: [PATCH 1/3] [http] use more efficient TMethodCall invocation When processing exe.json request, TMethodCall is used to invoke object method. Before all method arguments were packed into one string and provided as TMethodCall constructor. Main problem - interpreter involved every time and this leads to memory leak. Plus issue with escape symbols. Try to use different signature of TMethodCall::Execute where arguments parsed and stored as plain vector of pointers on these arguments. Such signature does not invoke interpreter and should avoid memory leak --- net/http/inc/TRootSniffer.h | 2 +- net/http/src/TRootSniffer.cxx | 5 +- net/httpsniff/src/TRootSnifferFull.cxx | 144 +++++++++++++++++++------ 3 files changed, 118 insertions(+), 33 deletions(-) diff --git a/net/http/inc/TRootSniffer.h b/net/http/inc/TRootSniffer.h index 756542e93a358..096226c3ac366 100644 --- a/net/http/inc/TRootSniffer.h +++ b/net/http/inc/TRootSniffer.h @@ -144,7 +144,7 @@ class TRootSniffer : public TNamed { virtual void ScanRoot(TRootSnifferScanRec &rec); - TString DecodeUrlOptionValue(const char *value, Bool_t remove_quotes = kTRUE); + TString DecodeUrlOptionValue(const char *value, Bool_t remove_quotes = kTRUE, Bool_t escape_special = kTRUE); TObject *GetItem(const char *fullname, TFolder *&parent, Bool_t force = kFALSE, Bool_t within_objects = kTRUE); diff --git a/net/http/src/TRootSniffer.cxx b/net/http/src/TRootSniffer.cxx index d99bf5fd180e2..6cd69b71f8639 100644 --- a/net/http/src/TRootSniffer.cxx +++ b/net/http/src/TRootSniffer.cxx @@ -1319,7 +1319,7 @@ Bool_t TRootSniffer::ProduceXml(const std::string &/* path */, const std::string //////////////////////////////////////////////////////////////////////////////// /// Method replaces all kind of special symbols, which could appear in URL options -TString TRootSniffer::DecodeUrlOptionValue(const char *value, Bool_t remove_quotes) +TString TRootSniffer::DecodeUrlOptionValue(const char *value, Bool_t remove_quotes, Bool_t escape_special) { if (!value || !*value) return ""; @@ -1350,6 +1350,9 @@ TString TRootSniffer::DecodeUrlOptionValue(const char *value, Bool_t remove_quot res.Remove(0, 1); } + if (!escape_special && remove_quotes) + return res; + // we expect normal content here, no special symbols, no unescaped quotes TString clean; for (Ssiz_t i = 0; i < res.Length(); ++i) { diff --git a/net/httpsniff/src/TRootSnifferFull.cxx b/net/httpsniff/src/TRootSnifferFull.cxx index f7b5661014c2a..fb1bbcc056bb9 100644 --- a/net/httpsniff/src/TRootSnifferFull.cxx +++ b/net/httpsniff/src/TRootSnifferFull.cxx @@ -512,6 +512,29 @@ Bool_t TRootSnifferFull::ProduceXml(const std::string &path, const std::string & return !res.empty(); } +class TArgHolderBase : public TObject { + public: + TArgHolderBase() : TObject() {} + virtual const void *GetPtr() const { return nullptr; } +}; + +template +class TArgHolder : public TArgHolderBase { + public: + T fValue; + TArgHolder(T v) : fValue(v) {} + const void *GetPtr() const override { return &fValue; } +}; + +class TArgHolderConstChar : public TArgHolderBase { + public: + TString fValue; + const char *fBuf = nullptr; + TArgHolderConstChar(const char *v) : fValue(v) { fBuf = fValue.Data(); } + const void *GetPtr() const override { return &fBuf; } +}; + + //////////////////////////////////////////////////////////////////////////////// /// Execute command for specified object /// @@ -613,9 +636,18 @@ Bool_t TRootSnifferFull::ProduceExe(const std::string &path, const std::string & garbage.SetOwner(kTRUE); // use as garbage collection TObject *post_obj = nullptr; // object reconstructed from post request TString call_args; + std::vector plain_args; + Bool_t can_use_plain = kTRUE, add_plain = kFALSE; + + auto add_plain_arg = [&plain_args, &garbage, &add_plain](TArgHolderBase *arg) { + plain_args.emplace_back(arg->GetPtr()); + garbage.Add(arg); + add_plain = kTRUE; + }; TIter next(args); while (auto arg = static_cast(next())) { + add_plain = kFALSE; if ((strcmp(arg->GetName(), "rest_url_opt") == 0) && (strcmp(arg->GetFullTypeName(), "const char*") == 0) && (args->GetSize() == 1)) { @@ -629,6 +661,7 @@ Bool_t TRootSnifferFull::ProduceExe(const std::string &path, const std::string & call_args.Append("\""); call_args.Append(DecodeUrlOptionValue(rest_url, kTRUE)); call_args.Append("\""); + add_plain_arg(new TArgHolderConstChar(rest_url)); break; } @@ -642,6 +675,7 @@ Bool_t TRootSnifferFull::ProduceExe(const std::string &path, const std::string & if (sval == "_this_") { // special case - object itself is used as argument sval.Form("(%s*)0x%zx", obj_cl->GetName(), (size_t)obj_ptr); + add_plain_arg(new TArgHolder(obj_ptr)); } else if ((fCurrentArg != nullptr) && (fCurrentArg->GetPostData() != nullptr)) { // process several arguments which are specific for post requests if (fAllowPostObject && (sval == "_post_object_xml_")) { @@ -654,6 +688,7 @@ Bool_t TRootSnifferFull::ProduceExe(const std::string &path, const std::string & if (url.HasOption("_destroy_post_")) garbage.Add(post_obj); } + add_plain_arg(new TArgHolder(post_obj)); } else if (fAllowPostObject && (sval == "_post_object_json_")) { // post data has extra 0 at the end and can be used as null-terminated string post_obj = TBufferJSON::ConvertFromJSON((const char *)fCurrentArg->GetPostData()); @@ -664,6 +699,7 @@ Bool_t TRootSnifferFull::ProduceExe(const std::string &path, const std::string & if (url.HasOption("_destroy_post_")) garbage.Add(post_obj); } + add_plain_arg(new TArgHolder(post_obj)); } else if (fAllowPostObject && (sval == "_post_object_") && url.HasOption("_post_class_")) { TString clname = DecodeUrlOptionValue(url.GetValueFromOptions("_post_class_"), kTRUE); TClass *arg_cl = gROOT->GetClass(clname, kTRUE, kTRUE); @@ -686,9 +722,11 @@ Bool_t TRootSnifferFull::ProduceExe(const std::string &path, const std::string & sval = "0"; else sval.Form("(%s*)0x%zx", clname.Data(), (size_t)post_obj); - } else if (sval == "_post_data_") + add_plain_arg(new TArgHolder(post_obj)); + } else if (sval == "_post_data_") { sval.Form("(void*)0x%zx", (size_t)fCurrentArg->GetPostData()); - else if (sval == "_post_length_") + add_plain_arg(new TArgHolder(fCurrentArg->GetPostData())); + } else if (sval == "_post_length_") sval.Form("%ld", (long)fCurrentArg->GetPostDataLength()); else sanitize_numeric = kTRUE; @@ -706,6 +744,39 @@ Bool_t TRootSnifferFull::ProduceExe(const std::string &path, const std::string & if (call_args.Length() > 0) call_args += ", "; + if (!add_plain) { + std::string tname = arg->GetTypeNormalizedName(); + if (tname == "const char*") + // one can use original string, just remove optional quotes + add_plain_arg(new TArgHolderConstChar(DecodeUrlOptionValue(val, kTRUE, kFALSE))); + else if (tname == "bool") + add_plain_arg(new TArgHolder(!sval.IsNull() && (sval != "0") && (sval != "false"))); + else if (tname == "double") + add_plain_arg(new TArgHolder(std::stod(sval.Data()))); + else if (tname == "float") + add_plain_arg(new TArgHolder(std::stof(sval.Data()))); + else if (tname == "int") + add_plain_arg(new TArgHolder(std::stol(sval.Data()))); + else if (tname == "long") + add_plain_arg(new TArgHolder(std::stol(sval.Data()))); + else if (tname == "short") + add_plain_arg(new TArgHolder(std::stol(sval.Data()))); + else if (tname == "char") + add_plain_arg(new TArgHolder(std::stol(sval.Data()))); + else if (tname == "unsigned int") + add_plain_arg(new TArgHolder(std::stoul(sval.Data()))); + else if (tname == "unisgned long") + add_plain_arg(new TArgHolder(std::stoul(sval.Data()))); + else if (tname == "unsigned short") + add_plain_arg(new TArgHolder(std::stoul(sval.Data()))); + else if (tname == "unsigned char") + add_plain_arg(new TArgHolder(std::stoul(sval.Data()))); + else if (!tname.empty() && tname.back() == '*' && (sval == "0" || sval == "null" || sval == "nullptr")) + add_plain_arg(new TArgHolder(0)); + else + can_use_plain = kFALSE; // unsupported type, plain args cannot be used + } + Bool_t isstr = (strcmp(arg->GetFullTypeName(), "const char*") == 0) || (strcmp(arg->GetFullTypeName(), "Option_t*") == 0) || (strcmp(arg->GetFullTypeName(), "string") == 0); @@ -742,7 +813,11 @@ Bool_t TRootSnifferFull::ProduceExe(const std::string &path, const std::string & TMethodCall *call = nullptr; if (method != nullptr) { - call = new TMethodCall(obj_cl, method_name, call_args.Data()); + if (can_use_plain) { + call = new TMethodCall(); + call->InitWithPrototype(obj_cl, method_name, prototype.IsNull() ? nullptr : prototype.Data()); + } else + call = new TMethodCall(obj_cl, method_name, call_args.Data()); if (debug) debug->append(TString::Format("Calling obj->%s(%s);\n", method_name, call_args.Data()).Data()); } else { @@ -772,10 +847,23 @@ Bool_t TRootSnifferFull::ProduceExe(const std::string &path, const std::string & garbage.Add(resbuf); } - switch (call->ReturnType()) { + auto ret_type = call->ReturnType(); + if (ret_type == TMethodCall::kOther) { + std::string ret_kind = func ? func->GetReturnTypeNormalizedName() : method->GetReturnTypeNormalizedName(); + if ((ret_kind.length() > 0) && (ret_kind[ret_kind.length() - 1] == '*')) { + ret_kind.resize(ret_kind.length() - 1); + ret_cl = gROOT->GetClass(ret_kind.c_str(), kTRUE, kTRUE); + } + if (!ret_cl) + ret_type = TMethodCall::kNone; + } + + switch (ret_type) { case TMethodCall::kLong: { - Longptr_t l(0); - if (method) + Longptr_t l = 0; + if (method && can_use_plain) + call->Execute(obj_ptr, plain_args.data(), plain_args.size(), &l); + else if (method) call->Execute(obj_ptr, l); else call->Execute(l); @@ -786,8 +874,10 @@ Bool_t TRootSnifferFull::ProduceExe(const std::string &path, const std::string & break; } case TMethodCall::kDouble: { - Double_t d(0.); - if (method) + Double_t d = 0.; + if (method && can_use_plain) + call->Execute(obj_ptr, plain_args.data(), plain_args.size(), &d); + else if (method) call->Execute(obj_ptr, d); else call->Execute(d); @@ -799,7 +889,9 @@ Bool_t TRootSnifferFull::ProduceExe(const std::string &path, const std::string & } case TMethodCall::kString: { char *txt = nullptr; - if (method) + if (method && can_use_plain) + call->Execute(obj_ptr, plain_args.data(), plain_args.size(), &txt); + else if (method) call->Execute(obj_ptr, &txt); else call->Execute(&txt); @@ -812,31 +904,21 @@ Bool_t TRootSnifferFull::ProduceExe(const std::string &path, const std::string & break; } case TMethodCall::kOther: { - std::string ret_kind = func ? func->GetReturnTypeNormalizedName() : method->GetReturnTypeNormalizedName(); - if ((ret_kind.length() > 0) && (ret_kind[ret_kind.length() - 1] == '*')) { - ret_kind.resize(ret_kind.length() - 1); - ret_cl = gROOT->GetClass(ret_kind.c_str(), kTRUE, kTRUE); - } - - if (ret_cl != nullptr) { - Longptr_t l(0); - if (method) - call->Execute(obj_ptr, l); - else - call->Execute(l); - if (l != 0) - ret_obj = (void *)l; - } else { - if (method) - call->Execute(obj_ptr); - else - call->Execute(); - } - + Longptr_t l = 0; + if (method && can_use_plain) + call->Execute(obj_ptr, plain_args.data(), plain_args.size(), &l); + else if (method) + call->Execute(obj_ptr, l); + else + call->Execute(l); + if (l != 0) + ret_obj = (void *)l; break; } case TMethodCall::kNone: { - if (method) + if (method && can_use_plain) + call->Execute(obj_ptr, plain_args.data(), plain_args.size()); + else if (method) call->Execute(obj_ptr); else call->Execute(); From 34add66c235fc1620ca58487ecbed07b0d86777a Mon Sep 17 00:00:00 2001 From: Sergey Linev Date: Wed, 10 Jun 2026 10:12:34 +0200 Subject: [PATCH 2/3] [http] keep cache for TMethodCall objects When exe.json executed for same method many times create TMethodCall once and reuse it. Should significantly reduce memory fragmentation from cling and probably avoid memory leaks. --- net/httpsniff/inc/TRootSnifferFull.h | 5 +++++ net/httpsniff/src/TRootSnifferFull.cxx | 20 +++++++++++++++----- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/net/httpsniff/inc/TRootSnifferFull.h b/net/httpsniff/inc/TRootSnifferFull.h index 7af2137646a7c..e4d56394ad1ad 100644 --- a/net/httpsniff/inc/TRootSnifferFull.h +++ b/net/httpsniff/inc/TRootSnifferFull.h @@ -14,13 +14,18 @@ #include "TRootSniffer.h" #include +#include +#include class TMemFile; +class TMethodCall; +class TMethod; class TRootSnifferFull : public TRootSniffer { protected: TMemFile *fMemFile{nullptr}; ///> fExeCache; ///InitWithPrototype(obj_cl, method_name, prototype.IsNull() ? nullptr : prototype.Data()); - } else + // prevent creation of huge cache + if (fExeCache.size() > 1000) + fExeCache.clear(); + auto iter = fExeCache.find(method); + if (iter != fExeCache.end()) { + call = iter->second.get(); + } else { + call = new TMethodCall(); + call->InitWithPrototype(obj_cl, method_name, prototype.IsNull() ? nullptr : prototype.Data()); + fExeCache.emplace(method, std::unique_ptr(call)); + } + } else { call = new TMethodCall(obj_cl, method_name, call_args.Data()); + garbage.Add(call); + } if (debug) debug->append(TString::Format("Calling obj->%s(%s);\n", method_name, call_args.Data()).Data()); } else { call = new TMethodCall(funcname.Data(), call_args.Data()); + garbage.Add(call); if (debug) debug->append(TString::Format("Calling %s(%s);\n", funcname.Data(), call_args.Data()).Data()); } - garbage.Add(call); - if (!call->IsValid()) { if (debug) debug->append("Fail: invalid TMethodCall\n"); From da0af8d074afb07f9fff8cbbb349028e89d7cd4e Mon Sep 17 00:00:00 2001 From: Sergey Linev Date: Wed, 10 Jun 2026 10:15:10 +0200 Subject: [PATCH 3/3] [http] extend test_sniffer.cxx Add test of multiple invocation of SetTitle in exe.json. Checks that cache works properly --- net/httpsniff/test/test_sniffer.cxx | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/net/httpsniff/test/test_sniffer.cxx b/net/httpsniff/test/test_sniffer.cxx index 67bb57098d188..e4782439cd7b3 100644 --- a/net/httpsniff/test/test_sniffer.cxx +++ b/net/httpsniff/test/test_sniffer.cxx @@ -230,7 +230,7 @@ TEST(TRootSniffer, set_title) std::string res; sniffer.Produce("/obj", "exe.json", "method=SetTitle&title=NewTitle", res); - EXPECT_EQ(res, "null") << "return value of exe.json when methout return void"; + EXPECT_EQ(res, "null") << "return value of exe.json when method return void"; EXPECT_EQ(std::string("NewTitle"), obj.GetTitle()) << "compare object title with applied value"; res = ""; @@ -244,9 +244,31 @@ TEST(TRootSniffer, set_title) EXPECT_EQ(std::string("UrlStyleQuotedTitle"), obj.GetTitle()) << "compare object title with applied value"; res = ""; - sniffer.Produce("/obj", "exe.json", "method=SetTitle&title=Mail\"Formed\"Title", res); + sniffer.Produce("/obj", "exe.json", "method=SetTitle&title=Mal\"formed\"title", res); EXPECT_EQ(res, "null"); - EXPECT_EQ(std::string("Mail\"Formed\"Title"), obj.GetTitle()) << "compare object title with applied value"; + EXPECT_EQ(std::string("Mal\"formed\"title"), obj.GetTitle()) << "compare object title with applied value"; +} + +// changing object title many times +TEST(TRootSniffer, many_set_title) +{ + TNamed obj("obj", "title"); + + TRootSnifferFull sniffer("sniffer"); + // disable readonly to get method executed + sniffer.SetReadOnly(kFALSE); + + sniffer.RegisterObject("/", &obj); + + std::string res; + + for (int n = 0; n < 1000; ++n) { + std::string new_title = "NewTitle" + std::to_string(n); + + sniffer.Produce("/obj", "exe.json", "method=SetTitle&title=" + new_title, res); + EXPECT_EQ(res, "null") << "return value of exe.json when method return void"; + EXPECT_EQ(new_title, obj.GetTitle()) << "compare object title with applied value"; + } } // testing command execution with different signatures