From 84ab2c064d8c41776088c037a4518a70b0f8e09d Mon Sep 17 00:00:00 2001
From: Philippe Canal <pcanal@fnal.gov>
Date: Thu, 6 Jun 2019 22:01:31 -0500
Subject: [PATCH] Insure that the commit callbacks don't change the 'current'
 transaction.

This should solve the problem we have been seeing with cmsUnload.  The problem stems from the fact that TCling::AutoParseImplRecurse
uses the address of the current transactions has an index/key when registering a class in fTransactionHeadersMap.

On some platforms TInterpreter.h is parsed late, i.e. during the compilation of:

   gInterpreter->AutoParse("SiStripCluster");

consequently (as AutoParse does not create its own transaction), the transactions that contains
the parsin of TInterpreter is the last one committed before AutoParseImplRecurse is executed.

Without the parsing of TInterpreter.h then that transaction is return by fInterpreter->getCurrentTransaction().
With the parsing of TInterpreter.h, during the commit of the transaction of the callbacks are triggered
and one of the decl is a constant (kFALSE) and thus, in TCling::HandleNewDecl, provokes the execution of

       if (gROOT->GetListOfGlobals()->FindObject(ND->getNameAsString().c_str()))
         return;

which will triggered the creation of a Transaction, however since the last top level transaction was
just switch to the state 'Committed', the new transaction will not be nested and thus
IncrementalParser::endTransaction will set the current transaction to 'nullptr'.

And this value of nullptr for the key of fTransactionHeadersMap is neither guaranteed to be unique
nor handled properly by the rest of the code.

It is unclear why this problem have surfaced recently.

On a failing machine, rebuilding with a 2 month old commit (ad9e5c42cb) still exhibits the problem.
---
 interpreter/cling/lib/Interpreter/IncrementalParser.cpp | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/interpreter/cling/lib/Interpreter/IncrementalParser.cpp b/interpreter/cling/lib/Interpreter/IncrementalParser.cpp
index 2a1724ffc31..02daa3d4ab5 100644
--- a/interpreter/cling/lib/Interpreter/IncrementalParser.cpp
+++ b/interpreter/cling/lib/Interpreter/IncrementalParser.cpp
@@ -629,9 +629,12 @@ namespace cling {
     }
     T->setState(Transaction::kCommitted);
 
-    if (InterpreterCallbacks* callbacks = m_Interpreter->getCallbacks())
-      callbacks->TransactionCommitted(*T);
-
+    {
+      Transaction* prevConsumerT = m_Consumer->getTransaction();
+      if (InterpreterCallbacks* callbacks = m_Interpreter->getCallbacks())
+        callbacks->TransactionCommitted(*T);
+      m_Consumer->setTransaction(prevConsumerT);
+    }
   }
 
   void IncrementalParser::emitTransaction(Transaction* T) {
-- 
GitLab