refactor: remove TH1::AddDirectory from DQMGenericClient #49575

gabrielmscampos · 2025-12-09T09:51:47Z

Retrieve FitSlicesY's results from TObjArray instead of current gDirectory.

PR description:

Resolves DQM's part in #49543 by removing TH1::AddDirectory and refactoring the code to retrieve histograms from a TObjArray.

PR validation:

cmsRun -j JobReport4.xml step4_HARVESTING.py

Retrieve FitSlicesY's results from TObjArray instead of current gDirectory.

cmsbuild · 2025-12-09T09:52:14Z

cms-bot internal usage

cmsbuild · 2025-12-09T09:54:00Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49575/47104

cmsbuild · 2025-12-09T09:54:22Z

A new Pull Request was created by @gabrielmscampos for master.

It involves the following packages:

DQMServices/Components (dqm)

@cmsbuild, @ctarricone, @gabrielmscampos, @nothingface0, @rseidita can you please review it and eventually sign? Thanks.
@barvic this is something you requested to watch as well.
@ftenchini, @mandrenguyen, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

gabrielmscampos · 2025-12-09T10:25:44Z

please test

cmsbuild · 2025-12-09T12:37:56Z

-1

Failed Tests: RelVals
Size: This PR adds an extra 28KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5fb5c5/49852/summary.html
COMMIT: 9eef0a9
CMSSW: CMSSW_16_0_X_2025-12-08-2300/el8_amd64_gcc13
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/49575/49852/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5fb5c5/49852/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5fb5c5/49852/git-merge-result

DAS Queries: The DAS query tests failed, see the summary page for details.

Failed RelVals

135.4A fatal system signal has occurred: segmentation violation
2025.0000001A fatal system signal has occurred: segmentation violation
34434.75A fatal system signal has occurred: segmentation violation

Expand to see more relval errors ...

…ctArray goes out of scope

cmsbuild · 2025-12-09T14:51:27Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49575/47113

cmsbuild · 2025-12-09T14:51:46Z

Pull request #49575 was updated. @cmsbuild, @ctarricone, @gabrielmscampos, @nothingface0, @rseidita can you please check and sign again.

gabrielmscampos · 2025-12-09T15:00:21Z

please test

Dr15Jones · 2025-12-09T17:03:36Z

DQMServices/Components/plugins/DQMGenericClient.cc

  TH1* h2;
  TH1* h3;
+
+  TH1* fetchSlice(TObjArray& arr, const std::string& name) {


This seems to leak memory. Might I suggest

Suggested change

TH1* fetchSlice(TObjArray& arr, const std::string& name) {

std::unique_ptr<TH1> fetchSlice(TObjArray& arr, const std::string& name) {

Hi @Dr15Jones, yes, I can implement your suggestion. May I ask why the first can lead to memory leak? Since it returns a raw pointer, the caller is responsible for deleting it (that is where I see the memory leak possibility), and in this case that deletion happens in the class destructor.

I had not looked at the scope for the variables being used. As CMS convention is to use m_' prefix or a _ suffix for member data names I assumed these were local variables. So it sounds like it isn't a memory leak. Might I suggest changing those member variables to be std::unique_ptr<> anyway and having the function still return std::unique_ptr?

Thanks. Done. I also took the opportunity to add the _ suffix to the member data names.

cmsbuild · 2025-12-10T08:25:39Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49575/47119

cmsbuild · 2025-12-10T08:26:05Z

Pull request #49575 was updated. @cmsbuild, @ctarricone, @gabrielmscampos, @nothingface0, @rseidita can you please check and sign again.

gabrielmscampos · 2025-12-10T08:29:05Z

please test

cmsbuild · 2025-12-10T10:02:53Z

+1

Size: This PR adds an extra 28KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5fb5c5/49874/summary.html
COMMIT: 17086f1
CMSSW: CMSSW_16_0_X_2025-12-09-2300/el8_amd64_gcc13
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/49575/49874/install.sh to create a dev area with all the needed externals and cmssw changes.

DAS Queries: The DAS query tests failed, see the summary page for details.

Comparison Summary

Summary:

You potentially added 2 lines to the logs
Reco comparison results: 6 differences found in the comparisons
Reco comparison had 4 failed jobs
DQMHistoTests: Total files compared: 53
DQMHistoTests: Total histograms compared: 4273241
DQMHistoTests: Total failures: 97
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 4273124
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 52 files compared)
Checked 227 log files, 198 edm output root files, 53 DQM output files
TriggerResults: no differences found

nothingface0 · 2025-12-11T09:44:39Z

+dqm

cmsbuild · 2025-12-11T09:45:05Z

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

mandrenguyen · 2025-12-15T09:47:08Z

+1

refactor: remove TH1::AddDirectory from DQMGenericClient

9eef0a9

Retrieve FitSlicesY's results from TObjArray instead of current gDirectory.

cmsbuild added this to the CMSSW_16_0_X milestone Dec 9, 2025

cmsbuild added dqm-pending pending-signatures tests-pending orp-pending code-checks-pending labels Dec 9, 2025

cmsbuild added code-checks-approved and removed code-checks-pending labels Dec 9, 2025

cmsbuild added tests-started and removed tests-pending labels Dec 9, 2025

cmsbuild added tests-rejected and removed tests-started labels Dec 9, 2025

fix: clone TH1 from TObject to avoid having it deleted when the TObje…

d282ce0

…ctArray goes out of scope

cmsbuild added tests-pending code-checks-pending and removed tests-rejected code-checks-approved labels Dec 9, 2025

cmsbuild added code-checks-approved and removed code-checks-pending labels Dec 9, 2025

cmsbuild added tests-started and removed tests-pending labels Dec 9, 2025

cmsbuild added the tests-approved label Dec 9, 2025

Dr15Jones reviewed Dec 9, 2025

View reviewed changes

gabrielmscampos added 2 commits December 10, 2025 09:12

style: add '_' suffix to data members to follow CMS convetion

27db91b

refactor: use smart pointer instead of raw pointer

17086f1

cmsbuild added tests-pending code-checks-pending and removed tests-approved code-checks-approved labels Dec 10, 2025

cmsbuild added code-checks-approved and removed code-checks-pending labels Dec 10, 2025

cmsbuild added tests-started and removed tests-pending labels Dec 10, 2025

cmsbuild added tests-approved and removed tests-started labels Dec 10, 2025

cmsbuild added dqm-approved fully-signed and removed dqm-pending pending-signatures labels Dec 11, 2025

cmsbuild added orp-approved and removed orp-pending labels Dec 15, 2025

cmsbuild merged commit 6bc7125 into cms-sw:master Dec 15, 2025
10 checks passed

This was referenced Dec 15, 2025

EXOnanoAODv1 producers and setup #48502

Open

Added Python Build Dependency for Openmpi cms-sw/cmsdist#10253

Merged

	TH1* fetchSlice(TObjArray& arr, const std::string& name) {
	std::unique_ptr<TH1> fetchSlice(TObjArray& arr, const std::string& name) {

refactor: remove TH1::AddDirectory from DQMGenericClient #49575

refactor: remove TH1::AddDirectory from DQMGenericClient #49575

Uh oh!

Conversation

gabrielmscampos commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR description:

PR validation:

Uh oh!

cmsbuild commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmsbuild commented Dec 9, 2025

Uh oh!

cmsbuild commented Dec 9, 2025

Uh oh!

gabrielmscampos commented Dec 9, 2025

Uh oh!

cmsbuild commented Dec 9, 2025

Failed RelVals

Uh oh!

cmsbuild commented Dec 9, 2025

Uh oh!

cmsbuild commented Dec 9, 2025

Uh oh!

gabrielmscampos commented Dec 9, 2025

Uh oh!

Dr15Jones Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

gabrielmscampos Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Dr15Jones Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

gabrielmscampos Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

cmsbuild commented Dec 10, 2025

Uh oh!

cmsbuild commented Dec 10, 2025

Uh oh!

gabrielmscampos commented Dec 10, 2025

Uh oh!

cmsbuild commented Dec 10, 2025

Comparison Summary

Uh oh!

nothingface0 commented Dec 11, 2025

Uh oh!

cmsbuild commented Dec 11, 2025

Uh oh!

mandrenguyen commented Dec 15, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gabrielmscampos commented Dec 9, 2025 •

edited

Loading

cmsbuild commented Dec 9, 2025 •

edited

Loading