-
Notifications
You must be signed in to change notification settings - Fork 54
Conversation
Segfault seems to be an issue with file and chain destructors:
When I remove the |
Even simple stuff is failing right now: >>> import ROOT
>>> f = ROOT.TFile()
>>> f
*** Break *** segmentation violation So this is some deeper issue with ROOT6 at the moment. |
@ndawe I've seen a segfault like this before. I know one of the things ROOT6 changed is memory management. In particular, one of the segfaults I encountered was deleting an object from memory while still using it later on, or deleting an object from memory and then ROOT segfaults because it expects to clean it up itself. |
Can you produce a stack trace or grab one? The current dump above doesn't give me enough information to dig through the code. |
@kratsg for my last post above? Simple TFile repr()? |
Yep. With the TFile. It shouldn't just crash like that. And as a proof of concept, can you just use something like |
Yeah, me too. Probably a problem on my end. Testing what you suggest. |
No problem with |
Oh, you know what? You might have multiple pythons running amok. If you use PyROOT compiled with ROOT5 against ROOT6 - it might muck up? I would double-check versioning here. EG: if you can check |
Hmm... I am pretty sure I only have ROOT6 setup. >>> import ROOT
>>> ROOT.__file__
'/home/endw/software/root/root_v6.02/lib/root/ROOT.pyc' This is interesting: >>> from ROOT import TFile
>>> f = TFile.Open('single1.root')
>>>
>>> f
<ROOT.TFile object ("single1.root") at 0x409a0d0>
>>>
>>> f = TFile()
>>> f
*** Break *** segmentation violation |
That is... huh. Let me confer the documentation for a minute because I'm going crazy. |
Both my ROOT5 and 6 builds are built against the same python and I only have python 2.7.8 on this system. |
I finally got a trace after letting gdb run at 100% for some time:
|
Ok, so two things.
The first line loads it, second line just checks to make sure you can do something stupid in python, and third line is the one that screwed you up before (and passes it into the ROOT interpreter). |
Yes, that all works fine (made minor changes to what you wrote). |
>>> ROOT.TTree()
<ROOT.TTree object at 0x2c49db0>
>>>
>>>
>>> ROOT.TChain()
<ROOT.TChain object at 0x2d26b60>
>>>
>>>
>>> ROOT.TFile()
*** Break *** segmentation violation |
That's magical. We should probably raise it up to the ROOT devs. This Friday, I'll be able to jump on to ROOT6 on the computer in my office and should be able to report back on the simple loads and see if I can reproduce this (on a Mac). Forum post is here: https://root.cern.ch/phpBB3/viewtopic.php?f=14&t=19097 |
OK great! Thanks! I'll take a break from this until tomorrow. |
Hmm... false alarm? That also crashes in 5.34.
I suppose I just never noticed that? |
... whoa. I can confirm the same. Python 2.7.8 and Root 5.34.
|
I just submitted a report to the ROOT devs here: |
Voted and am watching the issue. Updated the forum post with a link to the JIRA. I guess we're back to #165 (comment) now? |
Thanks! Yes, that was a dead end. Something else is up. |
For reference, I looked at:
Just looking at it, it seems like it should segfault as you're deleting the pointer, but not the memory allocated to it. It looks like a memory leak to me, but ROOT6 would catch these better than ROOT5 since ROOT6 will clean up a lot of stuff automatically. |
The dynamic_cast here in TFile.cxx must be failing:
|
Hmm. I think this is the correct way to delete a heap-allocated object in Cython. |
Yes. Must be some race condition. One of two/three possible crashes depending on if c.Delete() is there. |
I updated the ROOT bug report here: https://sft.its.cern.ch/jira/browse/ROOT-7015 with minimal code in Python and C++ that reproduces these crashes. ROOT 6 is broken indeed. |
@kratsg are you able to confirm a crash with this? #include "TChain.h"
int main() {
TChain* c = new TChain("tree");
c->Add("root_numpy/testdata/single1.root");
c->LoadTree(0);
c->AddBranchToCache("f_float");
c->SetCacheSize(1000000);
delete c;
} |
Can confirm crash with python code
|
Strange. What if you instead compile the C++? But it also crashes in the cling interpreter for me.
|
What commit are you on in ROOT? I compiled (and installed with --prefix) 09d1ffa42ccf27 (Jan 8). |
I'm on ROOT 5.34/23 (heads/v5-34-00-patches@v5-34-22-106-g4a0dea3. |
Right. This C++ doesn't crash in 5.X. Only 6. I also don't have it crashing in 5. |
I have ROOT6 on my other machine, but I'm not near it at the moment. I can test it tomorrow. |
That would be great. Thanks! |
@kratsg Philippe can now reproduce the crash and knows what's happening. Hopefully they have a fix soon. |
@ndawe I asked a friend of mine with ROOT6 - he can reproduce the crash. |
Waiting for https://sft.its.cern.ch/jira/browse/ROOT-7015 to be fixed before continuing here. Hopefully it's soon. |
Since we know that the problems above are on the ROOT side, I am going to merge this so that root_numpy at least compiles with C++11. |
https://sft.its.cern.ch/jira/browse/ROOT-7015 is fixed but there is a new issue: https://sft.its.cern.ch/jira/browse/ROOT-7200 |
This is hilarious and sad simultaneously. I'm curious about how ROOT6 is maintaining backwards-compatibility. I've run into the secondary issue you mentioned today on some of my own unit testing (JIRA#7200). |
Well, 7200 is apparently an intentional change, but I easily worked around it in root_numpy. Now we have another ROOT 6 issue: |
Include a possible
-std=c++11
when compiling, taken directly fromroot-config --cflags
.Currently building fine against ROOT6 locally, but I am sorting out what appears to be some segmentation violation that causes gdb to hang when running the tests.
The include path is part of
--cflags
so removal of--incdir
should be fine.