SND-LHC / sndsw

SND@LHC experiment framework based on FairShip and FairRoot
6 stars 16 forks source link

eventDisplay: Remove workaround for old ROOT versions #174

Closed olantwin closed 11 months ago

olantwin commented 11 months ago

As sndsw was never used with a ROOT version older than the one mentioned, it should be safe to remove this.

Note: it seems like this version check might be the wrong way round, it's true for versions larger than 6.07. Not sure whether this was intended.

I'm also confused, as I get a segmentation fault with and without this code when closing the Eve window manually and then exiting the python prompt.

ThomasRuf commented 11 months ago

do you also get a seg fault without manually closing TEve? If yes, leave the atexit and kill always the process. There are sum objects which are already deleted but ROOT still tries to delete them, which causes seg fault. Never figured out how to fix this, switched to the brute force method, and checked in newer root versions if problem disappeared.

olantwin commented 11 months ago

That's what I meant, when manually closing, I get the seg fault without and with the atexit, so for that case it doesn't make a difference.

When not closing Eve manually, without the atexit is closes fine, with the atexit it's terminated externally.

So it seems like the atexit is doing at best nothing in my case, and maybe hurts by interrupting the normal python exit.

ThomasRuf commented 11 months ago

not sure you answered my question. If you want to protect against a seg fault caused by manually closing TEve before python exit, you need an evExit() either without any conditions, or with a condition checking for this, and kill the process. Concerning the registering of evExit, there seems to be indeed an error > should become <.

olantwin commented 11 months ago

Currently the atexit is always active, because of the typo in the condition.

However, it does not help with the segfault when closing Eve (or at least is not sufficient). If the atexit helps for other crashes, we can or course leave it.

Concerning the registering of evExit, there seems to be indeed an error > should become <.

If that's the case, it should never be active in SNDSW, since ROOT 6.08 was released in 2016?

ThomasRuf commented 11 months ago

i think so. In any case, it only kills if: ROOT.gROOT.FindObject('Root Canvas EnergyLoss') . I think this is never the case for SND. If you want to protect against manually closing TEve window, youo need to add another condition. And of course register it.

olantwin commented 11 months ago

Ok, I'll test whether that makes a difference, and maybe update the version to include the current ROOT release, which still seems affected.

I'll close this PR and make a new one with the resulting changes.

olantwin commented 11 months ago

It seems like I can prevent false positives by using this check:

 if ROOT.addressof(ROOT.gEve) == 0:
  print("make suicide before framework makes seg fault")
  os.kill(os.getpid(),9)

But I think I have an idea of a better fix, now that I'm starting to understand the issue better.