-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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 pointers double deletions from PoolDBOutputService
in CondTools
subsystem
#35731
Fix pointers double deletions from PoolDBOutputService
in CondTools
subsystem
#35731
Conversation
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35731/26056
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
6a27c62
to
aef5077
Compare
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35731/26057
|
A new Pull Request was created by @mmusich (Marco Musich) for master. It involves the following packages:
@malbouis, @yuanchao, @cmsbuild, @AdrianoDee, @srimanob, @ggovi, @tvami, @francescobrivio can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild , please test |
@cmsbuild , ping |
CondTools/SiPhase2Tracker/plugins/SiPhase2OuterTrackerLorentzAngleReader.cc
Outdated
Show resolved
Hide resolved
aef5077
to
419cab1
Compare
Pull request #35731 was updated. @malbouis, @yuanchao, @cmsbuild, @AdrianoDee, @srimanob, @ggovi, @tvami, @francescobrivio can you please check and sign again. |
please test |
type bug-fix |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a21ba7/19740/summary.html Comparison SummarySummary:
|
+alca
|
+db |
@cms-sw/upgrade-l2 this fixes a couple of unit tests now broken in the IB: can you please review, and sign if you think so? |
+Upgrade For the upgrade part, |
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
resolves #35723 and #35670
PR description:
PR #35048 changed the behaviour of payload pointer ownership for condition updating workflows.
In a couple of workflows tested in unit tests (
CondTools/BeamSpot
andCondTools/SiPhase2Tracker
) the raw pointer underlying astd::unique_ptr
to the payload to be written was passed to thecreateNewIOV
andwriteOne
methods ofPoolDBOutputService
that are now deleting the pointer. This might lead to double deletion and consequent job crashes.In this PR (commit d68381f) the behaviour is changed by releasing the ownership of the managed payload object to the
PoolDBOutputService
hence avoiding the double deletion.I profit of this PR to introduce (in commit aef5077) a new plugin
SiPhase2OuterTrackerLorentzAngleReader
to readSiPhase2OuterTrackerLorentzAngle
payloads and use it in unit tests.PR validation:
Run the augmented unit tests
scram b runtests
if this PR is a backport please specify the original PR and why you need to backport that PR:
N/A