JFactoryPodioT: only delete shallow object in ClearData

[veprbl]

Resolves #241

[nathanwbrei]

I like this approach! Is this enough to make the heisenbug go away? I was a little worried that one of us would have to find a way to enforce that the podio::Frame gets deleted after all of the other mDatas.

[veprbl]

I think there is a small caveat here. I was focused on saving the case when mData is populated from objects managed by a collection, and did not appreciate that JANA in principle suggests that Set<> methods can be used too (which we don't do in EICrecon anymore). If it will be used, that would lead to a memory leak because of the unlink(). What is dubious to me is if an extended fix would have to account for the fact that Set/SetCollection append items to mData and not just sets them.

[nathanwbrei]

Set<> methods don't allow appending on PODIO objects because PODIO collections don't allow it once they've been added to the frame. One reason the code looks so weird is because the PODIO objects go through the exact same procedure, regardless of whether the user adds them to a collection themselves and calls SetCollection(), or calls Set<> and lets JANA do it behind the scenes. (Yes, the difference in constraints on Set<> annoys me, but the alternative is far worse)

TLDR; I think calling unlink() should be fine and won't cause a memory leak.

[veprbl]

Set<> methods don't allow appending on PODIO objects because PODIO collections don't allow it once they've been added to the frame.

It appends to mData on subsequent calls: https://github.com/veprbl/JANA2/blob/fad0866d147913759b50789573066b40eb0abc16/src/libraries/JANA/Podio/JFactoryPodioT.h#L184-L189

And I now see that the implementation of Insert() just ignores mData.

[veprbl]

Or rather Set doesn't append, but SetCollection does. So yeah, that's actually fine, I suppose.

[nathanwbrei]

Set<>() calls SetCollection(), which will fail on this->mFrame->put(std::move(collection), this->GetTag()); if this is an append. We probably should check that proactively though. Insert<>() forgetting to set mData is a bug though.