Skip to content

TOF: Fix out of scope shared ptr#920

Merged
Barthelemy merged 1 commit into
AliceO2Group:masterfrom
njacazio:nj-message-pad
Oct 22, 2021
Merged

TOF: Fix out of scope shared ptr#920
Barthelemy merged 1 commit into
AliceO2Group:masterfrom
njacazio:nj-message-pad

Conversation

@njacazio
Copy link
Copy Markdown
Collaborator

No description provided.

@njacazio
Copy link
Copy Markdown
Collaborator Author

@Barthelemy this is the fix for the issue, now the pad is a data member, memory leak will not be an issue anymore and the pad is kept in memory for future use

@Barthelemy
Copy link
Copy Markdown
Collaborator

This is better, thank you. My only fear is that the pad is destroyed by the histogram when it is itself destroyed. Could you confirm that the "functions" in the listoffunctions are not deleted when the histo is deleted ?

@njacazio
Copy link
Copy Markdown
Collaborator Author

Hi @Barthelemy I tested it and ran successfully, indeed it should not matter as the object is recreated if it was deleted in the meantime

@njacazio
Copy link
Copy Markdown
Collaborator Author

but maybe I could still check on th eobject itself e.g. with if (mMessagePad.get()) { // If the object is already created
@Barthelemy what do you think?

@Barthelemy
Copy link
Copy Markdown
Collaborator

the problem is that the internal pointer of mMessagePad might be deleted by the histogram without any way of knowing. You can see here that the deletion of the histogram provokes the deletion of all "functions" added to it, including your pad: https://root.cern.ch/doc/master/TH1_8cxx_source.html

I have had my fair share of problems with these "functions" in Run 1 and 2.

I am thinking that the simpler is perhaps to create the TPaveText each time MakeMessagePad is called and relying on the histogram to delete it might be the easiest. No smart pointers, just a raw pointer that you pass to the histo.

We rely of course on the fact that objects are transient and not kept in memory. Each time you get a new version, you add a fresh pave text.

@njacazio
Copy link
Copy Markdown
Collaborator Author

Ok, then I will revert to how it was before, sounds good!

@njacazio
Copy link
Copy Markdown
Collaborator Author

Hi @Barthelemy so this is the sumary:
A) I now check that I receive a message when I make the pad
B) I make the pad only if the message pad is enabled with a new every time and then it is the responsibility of the user/histogram to delete it

- Check that message is there before filling it
- Extend documentation
@Barthelemy Barthelemy merged commit 332ba02 into AliceO2Group:master Oct 22, 2021
Barthelemy pushed a commit that referenced this pull request Oct 22, 2021
- Check that message is there before filling it
- Extend documentation
@njacazio njacazio deleted the nj-message-pad branch March 18, 2022 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants