Cling: prevent double release of Transactions #152

smuzaffar · 2021-03-31T15:28:07Z

No description provided.

new of a char array might not have the correct alignment to hold a Transaction. Allocate a Transaction itself directly, instead of in-place constructing it in the character array. Each Transaction in the pool is thus constructed through `new Transaction(S)` and destructed through `delete T`, which is nicely symmetrical. The use of `::operator new` and `::operator delete` isn't actually necessary. While I'm at it, improve the assert message's wording.

smuzaffar · 2021-03-31T15:29:07Z

please test with cms-sw/cmsdist#6778

cmsbuild · 2021-03-31T15:31:14Z

A new Pull Request was created by @smuzaffar (Malik Shahzad Muzaffar) for branch cms/master/f078957.

@smuzaffar, @mrodozov can you please review it and eventually sign? Thanks.
cms-bot commands are listed here

cmsbuild · 2021-03-31T16:59:11Z

-1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-56e843/13892/summary.html
COMMIT: 734f63c
CMSSW: CMSSW_11_3_ROOT6_X_2021-03-30-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/root/152/13892/install.sh to create a dev area with all the needed externals and cmssw changes.

External Build

I found compilation error when building:

make[2]: *** Waiting for unfinished jobs....
make[2]: Leaving directory `/data/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/BUILD/slc7_amd64_gcc900/external/dd4hep/v01-16x-789ded0f3abbae7c552fa6d7a7d9bd76/build'
make[1]: *** [DDCore/CMakeFiles/DDCore.dir/all] Error 2
make[1]: Leaving directory `/data/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/BUILD/slc7_amd64_gcc900/external/dd4hep/v01-16x-789ded0f3abbae7c552fa6d7a7d9bd76/build'
make: *** [all] Error 2
error: Bad exit status from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/tmp/rpm-tmp.E0wJ12 (%build)


RPM build errors:
Macro %rpmbuild_libdir defined but not used within scope
Bad exit status from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/tmp/rpm-tmp.E0wJ12 (%build)

smuzaffar · 2021-03-31T17:06:29Z

please test with cms-sw/cmsdist#6778

This will allow Transaction remembering its RAII to not depend on Interpreter.

This will allow the Intrepreter to prevent unload() on Transactions held by RAIIs.

When a Transaction is unloaded for which *also* a ScopedTransactionRAII is waiting, the latter will potentially access a deleted Transaction. It can be deleted due to the Pool being full. Even if it is not deleted (as is the case in root-project#7657 ) it will get pushed into the Pool once by unload() and a second time by the RAII! The two options to track this case were full-blown ref counting or noting that a Transaction is attached to an RAII scope. If that is the case, freeing the Transaction must be skipped, as the RAII will take care of it.

a cheap way to notice what went wrong in root-project#7657.

cmsbuild · 2021-03-31T17:32:31Z

Pull request #152 was updated.

smuzaffar · 2021-03-31T18:14:58Z

please test with cms-sw/cmsdist#6778

cmsbuild · 2021-04-01T02:07:05Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-56e843/13901/summary.html
COMMIT: b9fa1a4
CMSSW: CMSSW_11_3_ROOT6_X_2021-03-30-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/root/152/13901/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 15 differences found in the comparisons
DQMHistoTests: Total files compared: 37
DQMHistoTests: Total histograms compared: 2640841
DQMHistoTests: Total failures: 13
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2640806
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 36 files compared)
Checked 155 log files, 37 edm output root files, 37 DQM output files
TriggerResults: no differences found

smuzaffar · 2021-04-01T05:36:24Z

please test with cms-sw/cmsdist#6778

smuzaffar · 2021-04-01T06:47:12Z

test parameters:

disable_poison = true

smuzaffar · 2021-04-01T06:48:04Z

please test with cms-sw/cmsdist#6778

cmsbuild · 2021-04-01T16:02:07Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-56e843/13916/summary.html
COMMIT: b9fa1a4
CMSSW: CMSSW_11_3_ROOT6_X_2021-03-31-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/root/152/13916/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 7 differences found in the comparisons
DQMHistoTests: Total files compared: 37
DQMHistoTests: Total histograms compared: 2640841
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2640818
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 36 files compared)
Checked 155 log files, 37 edm output root files, 37 DQM output files
TriggerResults: no differences found

cmsbuild added externals-pending orp-pending pending-signatures requires-external tests-started labels Mar 31, 2021

cmsbuild added tests-rejected and removed tests-started labels Mar 31, 2021

cmsbuild added tests-started and removed tests-rejected labels Mar 31, 2021

Axel-Naumann added 7 commits March 31, 2021 19:27

[cling] Move PushTransactionRAII out of Interpreter:

8349d74

This will allow Transaction remembering its RAII to not depend on Interpreter.

[cling] Order includes alphabetically (NFC).

caca1c4

[cling] Transaction needs to know its RAII:

8a19e55

This will allow the Intrepreter to prevent unload() on Transactions held by RAIIs.

[cling] Assert that no nested attaches to unloading Transaction.

290a31c

[cling] Remove historical leftover in TransactionPool size (NFC).

553d4f4

[cling] Assert released transaction is != last in pool:

b9fa1a4

a cheap way to notice what went wrong in root-project#7657.

Axel-Naumann force-pushed the issue-7657 branch from 734f63c to b9fa1a4 Compare March 31, 2021 17:32

cmsbuild added tests-pending and removed requires-external tests-started labels Mar 31, 2021

cmsbuild added requires-external tests-started and removed tests-pending labels Mar 31, 2021

smuzaffar mentioned this pull request Mar 31, 2021

PyROOT in root 6.24 branch hangs while loading CMSSW library root-project/root#7718

Closed

cmsbuild added tests-approved and removed tests-started labels Apr 1, 2021

cmsbuild added tests-started and removed tests-approved labels Apr 1, 2021

cmsbuild added tests-approved and removed tests-started labels Apr 1, 2021

smuzaffar closed this Apr 2, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cling: prevent double release of Transactions #152

Cling: prevent double release of Transactions #152

Uh oh!

smuzaffar commented Mar 31, 2021

Uh oh!

smuzaffar commented Mar 31, 2021

Uh oh!

cmsbuild commented Mar 31, 2021

Uh oh!

cmsbuild commented Mar 31, 2021

Uh oh!

smuzaffar commented Mar 31, 2021

Uh oh!

cmsbuild commented Mar 31, 2021

Uh oh!

smuzaffar commented Mar 31, 2021

Uh oh!

cmsbuild commented Apr 1, 2021

Uh oh!

smuzaffar commented Apr 1, 2021

Uh oh!

smuzaffar commented Apr 1, 2021

Uh oh!

smuzaffar commented Apr 1, 2021

Uh oh!

cmsbuild commented Apr 1, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Cling: prevent double release of Transactions #152

Cling: prevent double release of Transactions #152

Uh oh!

Conversation

smuzaffar commented Mar 31, 2021

Uh oh!

smuzaffar commented Mar 31, 2021

Uh oh!

cmsbuild commented Mar 31, 2021

Uh oh!

cmsbuild commented Mar 31, 2021

External Build

Uh oh!

smuzaffar commented Mar 31, 2021

Uh oh!

cmsbuild commented Mar 31, 2021

Uh oh!

smuzaffar commented Mar 31, 2021

Uh oh!

cmsbuild commented Apr 1, 2021

Comparison Summary

Uh oh!

smuzaffar commented Apr 1, 2021

Uh oh!

smuzaffar commented Apr 1, 2021

Uh oh!

smuzaffar commented Apr 1, 2021

Uh oh!

cmsbuild commented Apr 1, 2021

Comparison Summary

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants