Skip to content

[core] Add a stub for proper re-reading of rdicts if the library is reloaded. #5420

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
vgvassilev wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

[core] Add a stub for proper re-reading of rdicts if the library is reloaded. #5420

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions core/base/inc/TROOT.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,8 @@ friend TROOT *ROOT::Internal::GetROOT2();
const FwdDeclArgsToKeepCollection_t& fwdDeclsArgToSkip,
const char** classesHeaders,
bool hasCxxModule = false);
static void UnRegisterModule(const char* modulename,
void (*triggerFunc)());
TObject *Remove(TObject*);
void RemoveClass(TClass *);
void Reset(Option_t *option="");
Expand Down
12 changes: 12 additions & 0 deletions core/base/src/TROOT.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2567,6 +2567,18 @@ void TROOT::RegisterModule(const char* modulename,
}
}

////////////////////////////////////////////////////////////////////////////////
/// Called by static dictionary deinitialization to deregister clang modules
/// for headers. Calls TCling::UnRegisterModule().

void TROOT::UnRegisterModule(const char* modulename, void (*triggerFunc)())
{
if (TROOT::Initialized())
gCling->UnRegisterModule(modulename, triggerFunc);
// FIXME: Handle the deinitialization properly. ~TCling should call in
// reverse order UnRegisterModule when it is shutting down.
}

////////////////////////////////////////////////////////////////////////////////
/// Remove an object from the in-memory list.
/// Since TROOT is global resource, this is lock protected.
Expand Down
4 changes: 4 additions & 0 deletions core/dictgen/src/TModuleGenerator.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,10 @@ void TModuleGenerator::WriteRegistrationSourceImpl(std::ostream& out,
" DictInit() {\n"
" TriggerDictionaryInitialization_" << dictName << "_Impl();\n"
" }\n"
" ~DictInit() {\n"
" TROOT::UnRegisterModule(\"" << demangledDictName << "\","
"TriggerDictionaryInitialization_" << dictName << "_Impl);\n"
Copy link
Member

@pcanal pcanal Apr 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose that this is misleading (albeit harmeless) as you would a want a UnTrigger function here.

Copy link
Member Author

@vgvassilev vgvassilev Apr 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use that function to find back the shared object. We can avoid that by keeping a mapping in TCling between modulename and shared object. Then this function will take only one argument.

Copy link
Member

@pcanal pcanal Apr 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair enough. thanks.

Copy link
Member Author

@vgvassilev vgvassilev Apr 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall I make that change?

Copy link
Member

@pcanal pcanal Apr 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer that (but feel free not too if you prefer to leave it as is).

Copy link
Member Author

@vgvassilev vgvassilev Apr 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pcanal, could you provide an implementation of TCling::UnLoadPCM?

" }\n"
" } __TheDictionaryInitializer;\n"
"}\n"
"void TriggerDictionaryInitialization_" << dictName << "() {\n"
Expand Down
2 changes: 2 additions & 0 deletions core/meta/inc/TInterpreter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,8 @@ class TInterpreter : public TNamed {
Bool_t lateRegistration = false,
Bool_t hasCxxModule = false) = 0;
virtual void AddAvailableIndentifiers(TSeqCollection&) = 0;
virtual void UnRegisterModule(const char* /*modulename*/,
void (* /*triggerFunc*/)()) = 0;
virtual void RegisterTClassUpdate(TClass *oldcl,DictFuncPtr_t dict) = 0;
virtual void UnRegisterTClassUpdate(const TClass *oldcl) = 0;
virtual Int_t SetClassSharedLibs(const char *cls, const char *libs) = 0;
Expand Down
103 changes: 87 additions & 16 deletions core/metacling/src/TCling.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1695,7 +1695,7 @@ void TCling::RegisterRdictForLoadPCM(const std::string &pcmFileNameFullPath, llv
////////////////////////////////////////////////////////////////////////////////
/// Tries to load a PCM from TFile; returns true on success.

void TCling::LoadPCMImpl(TFile &pcmFile)
void TCling::LoadPCMImpl(TFile &pcmFile) const
{
auto listOfKeys = pcmFile.GetListOfKeys();

Expand Down Expand Up @@ -1842,10 +1842,10 @@ void TCling::LoadPCM(std::string pcmFileNameFullPath)
llvm::StringRef pcmContent = pendingRdict->second;
TMemFile::ZeroCopyView_t range{pcmContent.data(), pcmContent.size()};
std::string RDictFileOpts = pcmFileNameFullPath + "?filetype=pcm";
TMemFile pcmMemFile(RDictFileOpts.c_str(), range);

fLoadedRdicts.push_back(new TMemFile(RDictFileOpts.c_str(), range));
TFile* pcmMemFile = fLoadedRdicts.back();
cling::Interpreter::PushTransactionRAII deserRAII(GetInterpreterImpl());
LoadPCMImpl(pcmMemFile);
LoadPCMImpl(*pcmMemFile);
fPendingRdicts.erase(pendingRdict);

return;
Expand All @@ -1865,8 +1865,55 @@ void TCling::LoadPCM(std::string pcmFileNameFullPath)
Fatal("LoadPCM", "The file %s is not a ROOT as was expected\n", pcmFileName.Data());
return;
}
TFile pcmFile(pcmFileName + "?filetype=pcm", "READ");
LoadPCMImpl(pcmFile);
fLoadedRdicts.push_back(new TFile(pcmFileName + "?filetype=pcm", "READ"));
TFile* pcmMemFile = fLoadedRdicts.back();
LoadPCMImpl(*pcmMemFile);
}

void TCling::UnLoadPCMImpl(const TFile& rdict)
{
// FIXME: Reverse the effect of LoadPCMImpl.
Copy link
Member Author

@vgvassilev vgvassilev May 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pcanal, I have put all things in place except for this ;)

Copy link
Member Author

@vgvassilev vgvassilev Feb 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pcanal, the fate of this PR depends on this.

Copy link
Member Author

@vgvassilev vgvassilev Mar 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pcanal, I see you were working in a similar area, maybe you can also look into this and we can land this PR.

}

void TCling::UnLoadPCM(std::string pcmFileNameFullPath)
{
SuspendAutoLoadingRAII autoloadOff(this);
SuspendAutoParsing autoparseOff(this);
assert(!pcmFileNameFullPath.empty());
assert(llvm::sys::path::is_absolute(pcmFileNameFullPath));

// Easier to work with the ROOT interfaces.
TString pcmFileName = pcmFileNameFullPath;

// FIXME: @pcanal: how to deinitialize the streamer info factory?
R__InitStreamerInfoFactory();

TDirectory::TContext ctxt;
llvm::SaveAndRestore<Int_t> SaveGDebug(gDebug);
if (gDebug > 5) {
gDebug -= 5;
::Info("TCling::UnLoadPCM", "Loading ROOT PCM %s", pcmFileName.Data());
} else {
gDebug = 0;
}

if (llvm::sys::fs::is_symlink_file(pcmFileNameFullPath))
pcmFileNameFullPath = ROOT::TMetaUtils::GetRealPath(pcmFileNameFullPath);

TFile* rdictToUnload = nullptr;
for (const auto& rdict : fLoadedRdicts) {
if (rdict->GetName() == pcmFileNameFullPath) {
rdictToUnload = rdict;
break;
}
}

assert(rdictToUnload && "Rdict not loaded!");

// FIXME: Restore the state to fPendingRdicts, where we will need to have
// enough information to create an TMemFile.

UnLoadPCMImpl(*rdictToUnload);
}

//______________________________________________________________________________
Expand Down Expand Up @@ -1998,6 +2045,20 @@ void TCling::ProcessClassesToUpdate()
}
}
}

static std::string GetRdictFullPath(const std::string &dyLibName,
const char* modulename)
{
llvm::SmallString<256> pcmFileNameFullPath(dyLibName);
// The path dyLibName might not be absolute. This can happen if dyLibName
// is linked to an executable in the same folder.
llvm::sys::fs::make_absolute(pcmFileNameFullPath);
llvm::sys::path::remove_filename(pcmFileNameFullPath);
llvm::sys::path::append(pcmFileNameFullPath,
ROOT::TMetaUtils::GetModuleFileName(modulename));
return pcmFileNameFullPath.str().str();
}

////////////////////////////////////////////////////////////////////////////////
/// Inject the module named "modulename" into cling; load all headers.
/// headers is a 0-terminated array of header files to #include after
Expand Down Expand Up @@ -2287,16 +2348,8 @@ void TCling::RegisterModule(const char* modulename,
}
}

if (gIgnoredPCMNames.find(modulename) == gIgnoredPCMNames.end()) {
llvm::SmallString<256> pcmFileNameFullPath(dyLibName);
// The path dyLibName might not be absolute. This can happen if dyLibName
// is linked to an executable in the same folder.
llvm::sys::fs::make_absolute(pcmFileNameFullPath);
llvm::sys::path::remove_filename(pcmFileNameFullPath);
llvm::sys::path::append(pcmFileNameFullPath,
ROOT::TMetaUtils::GetModuleFileName(modulename));
LoadPCM(pcmFileNameFullPath.str().str());
}
if (gIgnoredPCMNames.find(modulename) == gIgnoredPCMNames.end())
LoadPCM(GetRdictFullPath(dyLibName, modulename));

{ // scope within which diagnostics are de-activated
// For now we disable diagnostics because we saw them already at
Expand Down Expand Up @@ -2374,6 +2427,24 @@ void TCling::AddAvailableIndentifiers(TSeqCollection& Idents) {
}


////////////////////////////////////////////////////////////////////////////////
/// Clean up after a dictionary was deinitialized (happens on dlclose).
///

void TCling::UnRegisterModule(const char* modulename, void (*triggerFunc)())
{
std::string dyLibName = cling::DynamicLibraryManager::getSymbolLocation(triggerFunc);
assert(!llvm::sys::fs::is_symlink_file(dyLibName));

if (dyLibName.empty()) {
::Error("TCling::UnRegisterModule", "Dictionary trigger function for %s not found", modulename);
return;
}

if (gIgnoredPCMNames.find(modulename) == gIgnoredPCMNames.end())
UnLoadPCM(GetRdictFullPath(dyLibName, modulename));
}

////////////////////////////////////////////////////////////////////////////////
/// Register classes that already existed prior to their dictionary loading
/// and that already had a ClassInfo (and thus would not be refresh via
Expand Down
6 changes: 5 additions & 1 deletion core/metacling/src/TCling.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ class TCling final : public TInterpreter {
Bool_t lateRegistration = false,
Bool_t hasCxxModule = false);
virtual void AddAvailableIndentifiers(TSeqCollection& Idents);
void UnRegisterModule(const char* modulename, void (*triggerFunc)());
void RegisterTClassUpdate(TClass *oldcl,DictFuncPtr_t dict);
void UnRegisterTClassUpdate(const TClass *oldcl);

Expand Down Expand Up @@ -621,9 +622,12 @@ class TCling final : public TInterpreter {
void AddFriendToClass(clang::FunctionDecl*, clang::CXXRecordDecl*) const;

std::map<std::string, llvm::StringRef> fPendingRdicts;
std::vector<TFile*> fLoadedRdicts; // ROOT owns the TFile-s.
void RegisterRdictForLoadPCM(const std::string &pcmFileNameFullPath, llvm::StringRef *pcmContent);
void LoadPCM(std::string pcmFileNameFullPath);
void LoadPCMImpl(TFile &pcmFile);
void LoadPCMImpl(TFile &pcmFile) const;
void UnLoadPCM(std::string pcmFileNameFullPath);
void UnLoadPCMImpl(const TFile &pcmFile);

void InitRootmapFile(const char *name);
int ReadRootmapFile(const char *rootmapfile, TUniqueString* uniqueString = nullptr);
Expand Down