Skip to content

Commit d0e9eca

Browse files
linevlinev
committed
[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
1 parent da45f06 commit d0e9eca

3 files changed

Lines changed: 118 additions & 33 deletions

File tree

‎net/http/inc/TRootSniffer.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ class TRootSniffer : public TNamed {
144144

145145
virtual void ScanRoot(TRootSnifferScanRec &rec);
146146

147-
TString DecodeUrlOptionValue(const char *value, Bool_t remove_quotes = kTRUE);
147+
TString DecodeUrlOptionValue(const char *value, Bool_t remove_quotes = kTRUE, Bool_t escape_special = kTRUE);
148148

149149
TObject *GetItem(const char *fullname, TFolder *&parent, Bool_t force = kFALSE, Bool_t within_objects = kTRUE);
150150

‎net/http/src/TRootSniffer.cxx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1321,7 +1321,7 @@ Bool_t TRootSniffer::ProduceXml(const std::string &/* path */, const std::string
13211321
////////////////////////////////////////////////////////////////////////////////
13221322
/// Method replaces all kind of special symbols, which could appear in URL options
13231323

1324-
TString TRootSniffer::DecodeUrlOptionValue(const char *value, Bool_t remove_quotes)
1324+
TString TRootSniffer::DecodeUrlOptionValue(const char *value, Bool_t remove_quotes, Bool_t escape_special)
13251325
{
13261326
if (!value || !*value)
13271327
return "";
@@ -1352,6 +1352,9 @@ TString TRootSniffer::DecodeUrlOptionValue(const char *value, Bool_t remove_quot
13521352
res.Remove(0, 1);
13531353
}
13541354

1355+
if (!escape_special && remove_quotes)
1356+
return res;
1357+
13551358
// we expect normal content here, no special symbols, no unescaped quotes
13561359
TString clean;
13571360
for (Ssiz_t i = 0; i < res.Length(); ++i) {

‎net/httpsniff/src/TRootSnifferFull.cxx‎

Lines changed: 113 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,29 @@ Bool_t TRootSnifferFull::ProduceXml(const std::string &path, const std::string &
511511
return !res.empty();
512512
}
513513

514+
class TArgHolderBase : public TObject {
515+
public:
516+
TArgHolderBase() : TObject() {}
517+
virtual const void *GetPtr() const { return nullptr; }
518+
};
519+
520+
template<typename T>
521+
class TArgHolder : public TArgHolderBase {
522+
public:
523+
T fValue;
524+
TArgHolder(T v) : fValue(v) {}
525+
const void *GetPtr() const override { return &fValue; }
526+
};
527+
528+
class TArgHolderConstChar : public TArgHolderBase {
529+
public:
530+
TString fValue;
531+
const char *fBuf = nullptr;
532+
TArgHolderConstChar(const char *v) : fValue(v) { fBuf = fValue.Data(); }
533+
const void *GetPtr() const override { return &fBuf; }
534+
};
535+
536+
514537
////////////////////////////////////////////////////////////////////////////////
515538
/// Execute command for specified object
516539
///
@@ -612,9 +635,18 @@ Bool_t TRootSnifferFull::ProduceExe(const std::string &path, const std::string &
612635
garbage.SetOwner(kTRUE); // use as garbage collection
613636
TObject *post_obj = nullptr; // object reconstructed from post request
614637
TString call_args;
638+
std::vector<const void *> plain_args;
639+
Bool_t can_use_plain = kTRUE, add_plain = kFALSE;
640+
641+
auto add_plain_arg = [&plain_args, &garbage, &add_plain](TArgHolderBase *arg) {
642+
plain_args.emplace_back(arg->GetPtr());
643+
garbage.Add(arg);
644+
add_plain = kTRUE;
645+
};
615646

616647
TIter next(args);
617648
while (auto arg = static_cast<TMethodArg *>(next())) {
649+
add_plain = kFALSE;
618650

619651
if ((strcmp(arg->GetName(), "rest_url_opt") == 0) && (strcmp(arg->GetFullTypeName(), "const char*") == 0) &&
620652
(args->GetSize() == 1)) {
@@ -628,6 +660,7 @@ Bool_t TRootSnifferFull::ProduceExe(const std::string &path, const std::string &
628660
call_args.Append("\"");
629661
call_args.Append(DecodeUrlOptionValue(rest_url, kTRUE));
630662
call_args.Append("\"");
663+
add_plain_arg(new TArgHolderConstChar(rest_url));
631664
break;
632665
}
633666

@@ -641,6 +674,7 @@ Bool_t TRootSnifferFull::ProduceExe(const std::string &path, const std::string &
641674
if (sval == "_this_") {
642675
// special case - object itself is used as argument
643676
sval.Form("(%s*)0x%zx", obj_cl->GetName(), (size_t)obj_ptr);
677+
add_plain_arg(new TArgHolder<void*>(obj_ptr));
644678
} else if ((fCurrentArg != nullptr) && (fCurrentArg->GetPostData() != nullptr)) {
645679
// process several arguments which are specific for post requests
646680
if (fAllowPostObject && (sval == "_post_object_xml_")) {
@@ -653,6 +687,7 @@ Bool_t TRootSnifferFull::ProduceExe(const std::string &path, const std::string &
653687
if (url.HasOption("_destroy_post_"))
654688
garbage.Add(post_obj);
655689
}
690+
add_plain_arg(new TArgHolder<void*>(post_obj));
656691
} else if (fAllowPostObject && (sval == "_post_object_json_")) {
657692
// post data has extra 0 at the end and can be used as null-terminated string
658693
post_obj = TBufferJSON::ConvertFromJSON((const char *)fCurrentArg->GetPostData());
@@ -663,6 +698,7 @@ Bool_t TRootSnifferFull::ProduceExe(const std::string &path, const std::string &
663698
if (url.HasOption("_destroy_post_"))
664699
garbage.Add(post_obj);
665700
}
701+
add_plain_arg(new TArgHolder<void*>(post_obj));
666702
} else if (fAllowPostObject && (sval == "_post_object_") && url.HasOption("_post_class_")) {
667703
TString clname = DecodeUrlOptionValue(url.GetValueFromOptions("_post_class_"), kTRUE);
668704
TClass *arg_cl = gROOT->GetClass(clname, kTRUE, kTRUE);
@@ -685,9 +721,11 @@ Bool_t TRootSnifferFull::ProduceExe(const std::string &path, const std::string &
685721
sval = "0";
686722
else
687723
sval.Form("(%s*)0x%zx", clname.Data(), (size_t)post_obj);
688-
} else if (sval == "_post_data_")
724+
add_plain_arg(new TArgHolder<void*>(post_obj));
725+
} else if (sval == "_post_data_") {
689726
sval.Form("(void*)0x%zx", (size_t)fCurrentArg->GetPostData());
690-
else if (sval == "_post_length_")
727+
add_plain_arg(new TArgHolder<const void*>(fCurrentArg->GetPostData()));
728+
} else if (sval == "_post_length_")
691729
sval.Form("%ld", (long)fCurrentArg->GetPostDataLength());
692730
else
693731
sanitize_numeric = kTRUE;
@@ -705,6 +743,39 @@ Bool_t TRootSnifferFull::ProduceExe(const std::string &path, const std::string &
705743
if (call_args.Length() > 0)
706744
call_args += ", ";
707745

746+
if (!add_plain) {
747+
std::string tname = arg->GetTypeNormalizedName();
748+
if (tname == "const char*")
749+
// one can use original string, just remove optional quotes
750+
add_plain_arg(new TArgHolderConstChar(DecodeUrlOptionValue(val, kTRUE, kFALSE)));
751+
else if (tname == "bool")
752+
add_plain_arg(new TArgHolder<bool>(!sval.IsNull() && (sval != "0") && (sval != "false")));
753+
else if (tname == "double")
754+
add_plain_arg(new TArgHolder<double>(std::stod(sval.Data())));
755+
else if (tname == "float")
756+
add_plain_arg(new TArgHolder<float>(std::stof(sval.Data())));
757+
else if (tname == "int")
758+
add_plain_arg(new TArgHolder<int>(std::stol(sval.Data())));
759+
else if (tname == "long")
760+
add_plain_arg(new TArgHolder<long>(std::stol(sval.Data())));
761+
else if (tname == "short")
762+
add_plain_arg(new TArgHolder<short>(std::stol(sval.Data())));
763+
else if (tname == "char")
764+
add_plain_arg(new TArgHolder<char>(std::stol(sval.Data())));
765+
else if (tname == "unsigned int")
766+
add_plain_arg(new TArgHolder<unsigned int>(std::stoul(sval.Data())));
767+
else if (tname == "unisgned long")
768+
add_plain_arg(new TArgHolder<unsigned long>(std::stoul(sval.Data())));
769+
else if (tname == "unsigned short")
770+
add_plain_arg(new TArgHolder<unsigned short>(std::stoul(sval.Data())));
771+
else if (tname == "unsigned char")
772+
add_plain_arg(new TArgHolder<unsigned char>(std::stoul(sval.Data())));
773+
else if (!tname.empty() && tname.back() == '*' && (sval == "0" || sval == "null" || sval == "nullptr"))
774+
add_plain_arg(new TArgHolder<Longptr_t>(0));
775+
else
776+
can_use_plain = kFALSE; // unsupported type, plain args cannot be used
777+
}
778+
708779
Bool_t isstr = (strcmp(arg->GetFullTypeName(), "const char*") == 0) ||
709780
(strcmp(arg->GetFullTypeName(), "Option_t*") == 0) ||
710781
(strcmp(arg->GetFullTypeName(), "string") == 0);
@@ -741,7 +812,11 @@ Bool_t TRootSnifferFull::ProduceExe(const std::string &path, const std::string &
741812
TMethodCall *call = nullptr;
742813

743814
if (method != nullptr) {
744-
call = new TMethodCall(obj_cl, method_name, call_args.Data());
815+
if (can_use_plain) {
816+
call = new TMethodCall();
817+
call->InitWithPrototype(obj_cl, method_name, prototype.IsNull() ? nullptr : prototype.Data());
818+
} else
819+
call = new TMethodCall(obj_cl, method_name, call_args.Data());
745820
if (debug)
746821
debug->append(TString::Format("Calling obj->%s(%s);\n", method_name, call_args.Data()).Data());
747822
} else {
@@ -771,10 +846,23 @@ Bool_t TRootSnifferFull::ProduceExe(const std::string &path, const std::string &
771846
garbage.Add(resbuf);
772847
}
773848

774-
switch (call->ReturnType()) {
849+
auto ret_type = call->ReturnType();
850+
if (ret_type == TMethodCall::kOther) {
851+
std::string ret_kind = func ? func->GetReturnTypeNormalizedName() : method->GetReturnTypeNormalizedName();
852+
if ((ret_kind.length() > 0) && (ret_kind[ret_kind.length() - 1] == '*')) {
853+
ret_kind.resize(ret_kind.length() - 1);
854+
ret_cl = gROOT->GetClass(ret_kind.c_str(), kTRUE, kTRUE);
855+
}
856+
if (!ret_cl)
857+
ret_type = TMethodCall::kNone;
858+
}
859+
860+
switch (ret_type) {
775861
case TMethodCall::kLong: {
776-
Longptr_t l(0);
777-
if (method)
862+
Longptr_t l = 0;
863+
if (method && can_use_plain)
864+
call->Execute(obj_ptr, plain_args.data(), plain_args.size(), &l);
865+
else if (method)
778866
call->Execute(obj_ptr, l);
779867
else
780868
call->Execute(l);
@@ -785,8 +873,10 @@ Bool_t TRootSnifferFull::ProduceExe(const std::string &path, const std::string &
785873
break;
786874
}
787875
case TMethodCall::kDouble: {
788-
Double_t d(0.);
789-
if (method)
876+
Double_t d = 0.;
877+
if (method && can_use_plain)
878+
call->Execute(obj_ptr, plain_args.data(), plain_args.size(), &d);
879+
else if (method)
790880
call->Execute(obj_ptr, d);
791881
else
792882
call->Execute(d);
@@ -798,7 +888,9 @@ Bool_t TRootSnifferFull::ProduceExe(const std::string &path, const std::string &
798888
}
799889
case TMethodCall::kString: {
800890
char *txt = nullptr;
801-
if (method)
891+
if (method && can_use_plain)
892+
call->Execute(obj_ptr, plain_args.data(), plain_args.size(), &txt);
893+
else if (method)
802894
call->Execute(obj_ptr, &txt);
803895
else
804896
call->Execute(&txt);
@@ -811,31 +903,21 @@ Bool_t TRootSnifferFull::ProduceExe(const std::string &path, const std::string &
811903
break;
812904
}
813905
case TMethodCall::kOther: {
814-
std::string ret_kind = func ? func->GetReturnTypeNormalizedName() : method->GetReturnTypeNormalizedName();
815-
if ((ret_kind.length() > 0) && (ret_kind[ret_kind.length() - 1] == '*')) {
816-
ret_kind.resize(ret_kind.length() - 1);
817-
ret_cl = gROOT->GetClass(ret_kind.c_str(), kTRUE, kTRUE);
818-
}
819-
820-
if (ret_cl != nullptr) {
821-
Longptr_t l(0);
822-
if (method)
823-
call->Execute(obj_ptr, l);
824-
else
825-
call->Execute(l);
826-
if (l != 0)
827-
ret_obj = (void *)l;
828-
} else {
829-
if (method)
830-
call->Execute(obj_ptr);
831-
else
832-
call->Execute();
833-
}
834-
906+
Longptr_t l = 0;
907+
if (method && can_use_plain)
908+
call->Execute(obj_ptr, plain_args.data(), plain_args.size(), &l);
909+
else if (method)
910+
call->Execute(obj_ptr, l);
911+
else
912+
call->Execute(l);
913+
if (l != 0)
914+
ret_obj = (void *)l;
835915
break;
836916
}
837917
case TMethodCall::kNone: {
838-
if (method)
918+
if (method && can_use_plain)
919+
call->Execute(obj_ptr, plain_args.data(), plain_args.size());
920+
else if (method)
839921
call->Execute(obj_ptr);
840922
else
841923
call->Execute();

0 commit comments

Comments
 (0)