-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
…oaded. If we call TCling::Load on an already loaded library we dlclose and dlopen the library. However, currently we do not have a facility to 'reload'/undo the effect of TCling::LoadPCM. This patch adds a section in the dictionary for de-registration which is reverse to TCling::RegisterModule -- TCling::UnRegisterModule. It tracks down which library is being reloaded and does not trigger a re-read of the rdict pcm. The current stub is can be further expanded to undo the effects caused by TCling::RegisterModule and/or improve the TCling shutdown by running the interpreter-dependent shutdown of the dictionary.
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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(); | ||
|
|
||
|
|
@@ -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; | ||
|
|
@@ -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. | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @pcanal, I have put all things in place except for this ;)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @pcanal, the fate of this PR depends on this.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
| } | ||
|
|
||
| //______________________________________________________________________________ | ||
|
|
@@ -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 | ||
|
|
@@ -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 | ||
|
|
@@ -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 | ||
|
|
||
There was a problem hiding this comment.
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
UnTriggerfunction here.There was a problem hiding this comment.
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
modulenameand shared object. Then this function will take only one argument.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fair enough. thanks.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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?