Prevent copy construction of `TrackerGeometry` #41901

mmusich · 2023-06-08T07:44:43Z

PR description:

In response to #41895.

delete the TrackerGeometry copy constructor and assignment operator, and default the move constructor and assignment operator in order to avoid undefined behaviour (double delete, usually leading to a crash) .

PR validation:

cmssw compiles.

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

Not a backport, perhaps to be backported?

cmsbuild · 2023-06-08T07:51:54Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41901/35843

This PR adds an extra 24KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41901/35843/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41901/35843/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2023-06-08T08:08:31Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41901/35845

This PR adds an extra 28KB to repository

cmsbuild · 2023-06-08T08:08:55Z

A new Pull Request was created by @mmusich (Marco Musich) for master.

It involves the following packages:

Alignment/CommonAlignmentMonitor (alca)
CondTools/SiPhase2Tracker (upgrade, alca)
Geometry/TrackerGeometryBuilder (geometry)

@civanch, @Dr15Jones, @bsunanda, @makortel, @mdhildreth, @cmsbuild, @AdrianoDee, @srimanob, @saumyaphor4252, @tvami, @francescobrivio can you please review it and eventually sign? Thanks.
@pakhotin, @VourMa, @adewit, @JanFSchulte, @tocheng, @VinInn, @bsunanda, @tlampen, @mmusich, @mtosi, @fabiocos, @venturia this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

Alignment/CommonAlignmentMonitor/plugins/AlignmentStats.cc

perrotta · 2023-06-10T07:17:55Z

@mmusich after the merge of #41900, this PR needs a rebase

mmusich · 2023-06-10T08:41:21Z

@perrotta

after the merge of #41900, this PR needs a rebase

I did rebase, just in order to execute tests and see if there are other problems with this approach, though this PR as is, is not viable (see #41901 (comment) and following).

cmsbuild · 2023-06-10T08:45:53Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41901/35872

This PR adds an extra 16KB to repository

cmsbuild · 2023-06-21T07:38:49Z

-1

Failed Tests: Build
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-239e69/33281/summary.html
COMMIT: 1e1f397
CMSSW: CMSSW_13_2_X_2023-06-20-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/41901/33281/install.sh to create a dev area with all the needed externals and cmssw changes.

Build

I found compilation error when building:

                 from /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02790/el8_amd64_gcc11/cms/cmssw-patch/CMSSW_13_2_X_2023-06-20-1100/src/FWCore/Framework/interface/one/EDAnalyzer.h:24,
                 from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_13_2_X_2023-06-20-1100/src/Alignment/CommonAlignmentMonitor/plugins/AlignmentStats.h:4,
                 from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_13_2_X_2023-06-20-1100/src/Alignment/CommonAlignmentMonitor/plugins/AlignmentStats.cc:1:
/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02790/el8_amd64_gcc11/external/gcc/11.2.1-f9b9dfdd886f71cd63f5538223d8f161/include/c++/11.2.1/bits/unique_ptr.h: In instantiation of 'typename std::_MakeUniq<_Tp>::__single_object std::make_unique(_Args&& ...) [with _Tp = TrackerGeometry; _Args = {const TrackerGeometry&}; typename std::_MakeUniq<_Tp>::__single_object = std::unique_ptr]':
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_13_2_X_2023-06-20-1100/src/Alignment/CommonAlignmentMonitor/plugins/AlignmentStats.cc:91:57:   required from here
/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02790/el8_amd64_gcc11/external/gcc/11.2.1-f9b9dfdd886f71cd63f5538223d8f161/include/c++/11.2.1/bits/unique_ptr.h:962:30: error: use of deleted function 'TrackerGeometry::TrackerGeometry(const TrackerGeometry&)'
  962 |     { return unique_ptr<_Tp>(new _Tp(std::forward<_Args>(__args)...)); }
      |                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_13_2_X_2023-06-20-1100/src/Alignment/CommonAlignmentMonitor/plugins/AlignmentStats.h:13,
                 from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_13_2_X_2023-06-20-1100/src/Alignment/CommonAlignmentMonitor/plugins/AlignmentStats.cc:1:
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_13_2_X_2023-06-20-1100/src/Geometry/TrackerGeometryBuilder/interface/TrackerGeometry.h:59:3: note: declared here

mmusich · 2023-06-21T11:31:10Z

@cmsbuild, please test

let's see if it picks up the new IB

cmsbuild · 2023-06-21T14:50:42Z

-1

Failed Tests: UnitTests RelVals AddOn
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-239e69/33296/summary.html
COMMIT: 1e1f397
CMSSW: CMSSW_13_2_X_2023-06-20-2300/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/41901/33296/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test testRecoMETMETProducers had ERRORS

RelVals

25.0

----- Begin Fatal Exception 21-Jun-2023 16:10:50 CEST-----------------------
An exception of category 'PluginNotFound' occurred while
   [0] Constructing the EventProcessor
Exception Message:
Unable to find plugin 'Pythia8ConcurrentGeneratorFilter' in category 'CMS EDM Framework Module'. Please check spelling of name.
----- End Fatal Exception -------------------------------------------------

5.1

----- Begin Fatal Exception 21-Jun-2023 16:11:08 CEST-----------------------
An exception of category 'PluginNotFound' occurred while
   [0] Constructing the EventProcessor
Exception Message:
Unable to find plugin 'Pythia8ConcurrentGeneratorFilter' in category 'CMS EDM Framework Module'. Please check spelling of name.
----- End Fatal Exception -------------------------------------------------

135.4

----- Begin Fatal Exception 21-Jun-2023 16:11:08 CEST-----------------------
An exception of category 'PluginNotFound' occurred while
   [0] Constructing the EventProcessor
Exception Message:
Unable to find plugin 'Pythia8ConcurrentGeneratorFilter' in category 'CMS EDM Framework Module'. Please check spelling of name.
----- End Fatal Exception -------------------------------------------------

Expand to see more relval errors ...

AddOn Tests

fastsim

----- Begin Fatal Exception 21-Jun-2023 16:10:38 CEST-----------------------
An exception of category 'PluginNotFound' occurred while
   [0] Constructing the EventProcessor
Exception Message:
Unable to find plugin 'Pythia8ConcurrentGeneratorFilter' in category 'CMS EDM Framework Module'. Please check spelling of name.
----- End Fatal Exception -------------------------------------------------

fastsim1

----- Begin Fatal Exception 21-Jun-2023 16:10:38 CEST-----------------------
An exception of category 'PluginNotFound' occurred while
   [0] Constructing the EventProcessor
Exception Message:
Unable to find plugin 'Pythia8ConcurrentGeneratorFilter' in category 'CMS EDM Framework Module'. Please check spelling of name.
----- End Fatal Exception -------------------------------------------------

fastsim2

----- Begin Fatal Exception 21-Jun-2023 16:10:38 CEST-----------------------
An exception of category 'PluginNotFound' occurred while
   [0] Constructing the EventProcessor
Exception Message:
Unable to find plugin 'Pythia8ConcurrentGeneratorFilter' in category 'CMS EDM Framework Module'. Please check spelling of name.
----- End Fatal Exception -------------------------------------------------

Expand to see more addon errors ...

mmusich · 2023-06-22T06:15:26Z

@cmsbuild, please test

cmsbuild · 2023-06-22T09:27:10Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-239e69/33316/summary.html
COMMIT: 1e1f397
CMSSW: CMSSW_13_2_X_2023-06-21-2300/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/41901/33316/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially removed 19 lines from the logs
Reco comparison results: 2 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3200270
DQMHistoTests: Total failures: 3
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3200245
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
Checked 207 log files, 159 edm output root files, 48 DQM output files
TriggerResults: no differences found

civanch · 2023-06-24T11:40:57Z

+1

cmsbuild · 2023-06-24T11:41:22Z

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, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

perrotta · 2023-06-25T15:22:57Z

+1

cmsbuild added this to the CMSSW_13_2_X milestone Jun 8, 2023

cmsbuild added alca-pending geometry-pending pending-signatures tests-pending orp-pending upgrade-pending code-checks-pending trk labels Jun 8, 2023

mmusich mentioned this pull request Jun 8, 2023

update unit tests for CondTools/SiPhase2Tracker package to register failures and fix underlying code #41900

Merged

cmsbuild added code-checks-rejected and removed code-checks-pending labels Jun 8, 2023

mmusich force-pushed the TrackerGeometryCopyConstruct branch from de93849 to 49dde19 Compare June 8, 2023 07:57

cmsbuild added code-checks-pending and removed code-checks-rejected labels Jun 8, 2023

mmusich force-pushed the TrackerGeometryCopyConstruct branch from 49dde19 to 6486f5e Compare June 8, 2023 07:59

cmsbuild added code-checks-approved and removed code-checks-pending labels Jun 8, 2023

makortel reviewed Jun 8, 2023

View reviewed changes

Alignment/CommonAlignmentMonitor/plugins/AlignmentStats.cc Outdated Show resolved Hide resolved

mmusich force-pushed the TrackerGeometryCopyConstruct branch from 6486f5e to d65073f Compare June 10, 2023 08:37

cmsbuild added code-checks-pending and removed upgrade-pending code-checks-approved labels Jun 10, 2023

cmsbuild removed the code-checks-pending label Jun 10, 2023

cmsbuild added tests-started tests-rejected and removed tests-pending tests-started labels Jun 21, 2023

cmsbuild added tests-started and removed tests-rejected labels Jun 21, 2023

cmsbuild added tests-rejected and removed tests-started labels Jun 21, 2023

cmsbuild added tests-started and removed tests-rejected labels Jun 22, 2023

cmsbuild added tests-approved and removed tests-started labels Jun 22, 2023

cmsbuild added geometry-approved fully-signed and removed geometry-pending pending-signatures labels Jun 24, 2023

cmsbuild added orp-approved and removed orp-pending labels Jun 25, 2023

cmsbuild merged commit c728f8f into cms-sw:master Jun 25, 2023

mmusich deleted the TrackerGeometryCopyConstruct branch June 25, 2023 16:06

cmsbuild mentioned this pull request Jun 26, 2023

addition of 2023C WFs and modifications 2023B WFs to run with era Run3 #42069

Merged

mmusich mentioned this pull request Jun 26, 2023

TrackerGeometry default copy constructor is unsafe #41895

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent copy construction of `TrackerGeometry` #41901

Prevent copy construction of `TrackerGeometry` #41901

mmusich commented Jun 8, 2023 •

edited

Loading

cmsbuild commented Jun 8, 2023

cmsbuild commented Jun 8, 2023

cmsbuild commented Jun 8, 2023

perrotta commented Jun 10, 2023

mmusich commented Jun 10, 2023

cmsbuild commented Jun 10, 2023

cmsbuild commented Jun 21, 2023

mmusich commented Jun 21, 2023

cmsbuild commented Jun 21, 2023

mmusich commented Jun 22, 2023

cmsbuild commented Jun 22, 2023

civanch commented Jun 24, 2023

cmsbuild commented Jun 24, 2023

perrotta commented Jun 25, 2023

Prevent copy construction of TrackerGeometry #41901

Prevent copy construction of TrackerGeometry #41901

Conversation

mmusich commented Jun 8, 2023 • edited Loading

PR description:

PR validation:

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

cmsbuild commented Jun 8, 2023

cmsbuild commented Jun 8, 2023

cmsbuild commented Jun 8, 2023

perrotta commented Jun 10, 2023

mmusich commented Jun 10, 2023

cmsbuild commented Jun 10, 2023

cmsbuild commented Jun 21, 2023

Build

mmusich commented Jun 21, 2023

cmsbuild commented Jun 21, 2023

Unit Tests

RelVals

AddOn Tests

mmusich commented Jun 22, 2023

cmsbuild commented Jun 22, 2023

Comparison Summary

civanch commented Jun 24, 2023

cmsbuild commented Jun 24, 2023

perrotta commented Jun 25, 2023

Prevent copy construction of `TrackerGeometry` #41901

Prevent copy construction of `TrackerGeometry` #41901

mmusich commented Jun 8, 2023 •

edited

Loading