-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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.
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
Fix potential crash of the rreader test on Windows #13782
Conversation
Delete the `TFile` pointers, preventing a potential crash in `TROOT::CloseFiles()` when trying to call the `Close()` method on `TWebSocket`/`TWebFile` via the interpreter `CallFunc_Exec` on Windows (visble with LLVM 16)
Starting build on |
tmva/tmva/test/rreader.cxx
Outdated
@@ -53,6 +53,7 @@ void TrainClassificationModel() | |||
factory->BookMethod(dataloader, TMVA::Types::kBDT, "BDT", "!V:!H:NTrees=100:MaxDepth=2"); | |||
factory->TrainAllMethods(); | |||
output->Close(); | |||
delete data; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think wrapping the file in a unique pointer would be the preferred solution, like this:
std::unique_ptr<data>{TFile::Open(filename.c_str())};
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, thanks for the suggestion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing the memory leaks!
If you have time, please consider updating this to use smart pointers instead of raw pointers to TFile
.
Starting build on |
Build failed on mac11/noimt. Failing tests: |
Build failed on ROOT-ubuntu2004/python3. Errors:
|
* Fix potential crash of the rreader test on Windows Delete the `TFile` pointers, preventing a potential crash in `TROOT::CloseFiles()` when trying to call the `Close()` method on `TWebSocket`/`TWebFile` via the interpreter `CallFunc_Exec` on Windows (visble with LLVM 16) * Use `std::unique_ptr` instead of deleting the pointers (thanks Jonas)
Delete the
TFile
pointers, preventing a potential crash inTROOT::CloseFiles()
when trying to call theClose()
method onTWebSocket
/TWebFile
via the interpreterCallFunc_Exec
on Windows (visible with LLVM 16)