Skip to content

DPL: Fix leak in TTree plugin #13800

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ktf merged 1 commit into from
Dec 13, 2024
Merged

DPL: Fix leak in TTree plugin #13800

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions Framework/AnalysisSupport/src/TTreePlugin.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
const std::string& path,
const std::shared_ptr<const arrow::KeyValueMetadata>& metadata) override;

virtual TTree* GetTree(arrow::dataset::FileSource source) = 0;
virtual std::unique_ptr<TTree>& GetTree(arrow::dataset::FileSource source) = 0;
};

class SingleTreeFileSystem : public TTreeFileSystem
Expand All @@ -72,14 +72,14 @@
return "ttree";
}

TTree* GetTree(arrow::dataset::FileSource) override
std::unique_ptr<TTree>& GetTree(arrow::dataset::FileSource) override
{
// Simply return the only TTree we have
return mTree;
}

private:
TTree* mTree;
std::unique_ptr<TTree> mTree;
};

arrow::Result<arrow::fs::FileInfo> SingleTreeFileSystem::GetFileInfo(std::string const& path)
Expand Down Expand Up @@ -158,7 +158,9 @@
class TTreeOutputStream : public arrow::io::OutputStream
{
public:
TTreeOutputStream(TTree*, std::string branchPrefix);
// Using a pointer means that the tree itself is owned by another

Check failure on line 161 in Framework/AnalysisSupport/src/TTreePlugin.cxx

View workflow job for this annotation

GitHub Actions / PR formatting / whitespace

Trailing spaces 

Remove the trailing spaces at the end of the line.
// class
TTreeOutputStream(TTree *, std::string branchPrefix);

arrow::Status Close() override;

Expand Down Expand Up @@ -265,7 +267,7 @@
auto fs = std::dynamic_pointer_cast<TTreeFileSystem>(containerFS->GetSubFilesystem(treeFragment->source()));

int64_t rows = -1;
TTree* tree = fs->GetTree(treeFragment->source());
auto& tree = fs->GetTree(treeFragment->source());
for (auto& field : fields) {
// The field actually on disk
auto physicalField = physical_schema->GetFieldByName(field->name());
Expand Down Expand Up @@ -477,9 +479,9 @@
arrow::dataset::FileSource source{path, shared_from_this()};
auto prefix = metadata->Get("branch_prefix");
if (prefix.ok()) {
return std::make_shared<TTreeOutputStream>(GetTree(source), *prefix);
return std::make_shared<TTreeOutputStream>(GetTree(source).get(), *prefix);
}
return std::make_shared<TTreeOutputStream>(GetTree(source), "");
return std::make_shared<TTreeOutputStream>(GetTree(source).get(), "");
}

namespace
Expand Down Expand Up @@ -541,7 +543,7 @@
if (!treeFs.get()) {
throw runtime_error_f("Unknown filesystem %s\n", source.filesystem()->type_name().c_str());
}
TTree* tree = treeFs->GetTree(source);
auto& tree = treeFs->GetTree(source);

auto branches = tree->GetListOfBranches();
auto n = branches->GetEntries();
Expand Down Expand Up @@ -688,7 +690,7 @@
// We already have a tree stream, let's derive a new one
// with the destination_locator_.path as prefix for the branches
// This way we can multiplex multiple tables in the same tree.
auto tree = treeStream->GetTree();
auto* tree = treeStream->GetTree();
treeStream = std::make_shared<TTreeOutputStream>(tree, destination_locator_.path);
} else {
// I could simply set a prefix here to merge to an already existing tree.
Expand Down Expand Up @@ -834,7 +836,7 @@
arrow::Future<> FinishInternal() override
{
auto treeStream = std::dynamic_pointer_cast<TTreeOutputStream>(destination_);
TTree* tree = treeStream->GetTree();
auto* tree = treeStream->GetTree();
tree->Write("", TObject::kOverwrite);
tree->SetDirectory(nullptr);

Expand Down
Loading