From 63d4d9fd4dd50f00615f9d31cc2bc8696bb5bc6b Mon Sep 17 00:00:00 2001 From: Sergey Linev Date: Tue, 9 Jun 2026 17:14:26 +0200 Subject: [PATCH 1/2] [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 aa4368bba28d8..98fd761f12324 100644 --- a/net/http/src/TRootSniffer.cxx +++ b/net/http/src/TRootSniffer.cxx @@ -1321,7 +1321,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 ""; @@ -1352,6 +1352,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 8688a97cbcd20..f9802743d63c6 100644 --- a/net/httpsniff/src/TRootSnifferFull.cxx +++ b/net/httpsniff/src/TRootSnifferFull.cxx @@ -523,6 +523,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 /// @@ -624,9 +647,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)) { @@ -640,6 +672,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; } @@ -653,6 +686,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_")) { @@ -665,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_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()); @@ -675,6 +710,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); @@ -697,9 +733,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; @@ -717,6 +755,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); @@ -753,7 +824,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 { @@ -783,10 +858,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); @@ -797,8 +885,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); @@ -810,7 +900,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); @@ -823,31 +915,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 5db0c13b77644fb0f7b94eef4d82d199499fdeda Mon Sep 17 00:00:00 2001 From: Sergey Linev Date: Wed, 10 Jun 2026 10:12:34 +0200 Subject: [PATCH 2/2] [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");