Fixed segmentation fault in TrueJet_Parser

[nVentis]

BEGINRELEASENOTES

• Added missing pointer assignment to TrueJet_Parser to avoid segmentation faults

ENDRELEASENOTES

[jmcarcell]

Can you find where the pointers are being used after being deleted and fix that? There may be some bigger issue on why that's happening instead of relying on checking for NULL.

[nVentis]

The edit is sufficient, because the objects to be deleted are only used by TrueJet_Parser::getall() or methods called by it.

The rather trivial problem that is fixed by this PR comes from the fact that getall() may return before setting new objects. The delall() method intended to be called by the user at the end of each event for cleanup then attempts to call delete on the pointers again, resulting in undefined behavior (in my experience, segmentation faults).

[tmadlener]

Suggested change

	if ( relfcn!= NULL ) { delete relfcn; relfcn = NULL; }
	if ( relicn!= NULL ) { delete relicn; relicn = NULL; }
	if ( relfp!= NULL ) { delete relfp; relfp = NULL; }
	if ( relip!= NULL ) { delete relip; relip = NULL; }
	if ( reltjreco != NULL) { delete reltjreco; reltjreco = NULL; }
	if ( reltjmcp != NULL) { delete reltjmcp; reltjmcp = NULL; }
	if ( jets != NULL) { delete jets; jets = NULL; }
	if ( finalcns != NULL) { delete finalcns; finalcns = NULL; }
	if ( initialcns!= NULL ) { delete initialcns; initialcns = NULL; }
	if ( reltrue_tj != NULL ) { delete reltrue_tj; reltrue_tj = NULL; }
	delete relfcn; relfcn = nullptr;
	delete relicn; relicn = nullptr;
	delete relfp; relfp = nullptr;
	delete relip; relip = nullptr;
	delete reltjreco; reltjreco = nullptr;
	delete reltjmcp; reltjmcp = nullptr;
	delete jets; jets = nullptr;
	delete finalcns; finalcns = nullptr;
	delete initialcns; initialcns = nullptr;
	delete reltrue_tj; reltrue_tj = nullptr;

The important bit is setting to nullptr. Deleting a nullptr is defined and safe behavior per the c++ standard.

(Since we had this exact discussion yesterday in another context).

[jmcarcell]

Then the proper fix is to turn all these into std::unique_ptrs, they are all members of the class and used only there and deleted at the end? Then unique_ptr does exactly what the code is meant to do, it would delete every time it is assigned and when the class is destroyed.

[tmadlener]

I think this is a classic "fortran c++" program, where the user side looks something like this, i think

TrueJet_Parser parser;

for (int i = 0; i < nEvents; ++i) {
  auto event = // from wherever;
  parser.getall(event);
  // do whatever you do
  parser.delall();
}

So turning these into unique_ptrs would require a bit more work than this.

[jmcarcell]

But many of these are only used there inside the class. #58 should be exactly the same.