-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Issue exception for ROOT file forward incompatiblity problem [10_6] #43902
Conversation
This assert was happening when an old release tried to read a file written by a newer release.
An unzip error in the StreamerInfo appears to generate a nullptr without issuing an ROOT Error message.
please test |
A new Pull Request was created by @Dr15Jones (Chris Jones) for CMSSW_10_6_X. It involves the following packages:
@smuzaffar, @Dr15Jones, @makortel can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
cms-bot internal usage |
} | ||
if (file_->IsZombie()) { | ||
file_ = nullptr; // propagate_const<T> has no reset() function | ||
return; | ||
throw edm::Exception(errors::FileOpenError) << "TFile::Open returned zombie."; |
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.
Should we do these changes in master
too (irrespective of this issue at hand)?
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.
Should we do these changes in
master
too (irrespective of this issue at hand)?
@makortel
What is the conclusion to this question? Thanks.
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 put the changes in master as well.
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.
The master
PR is #43920
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
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-fb8f96/37289/summary.html Comparison SummarySummary:
|
+core |
This pull request is fully signed and it will be integrated in one of the next CMSSW_10_6_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_14_1_X is complete. This pull request will now be reviewed by the release team before it's merged. @sextonkennedy, @antoniovilela, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
PR validation:
This code was tested in CMSSW_10_6_30 and even with the backport seg faulted when reading a file generated in CMSSW_13_3. The problem was traced down to ROOT silently ignoring a failure while unzipping StreamerInfo. Added additional check on nullptr from TFile::Open gives a graceful exit to the job from an exception.
If this PR is a backport please specify the original PR and why you need to backport that PR.
Part of large scale backport of #43894
fixes cms-sw/framework-team#804