Skip to content

Commit 1b894b3

Browse files
committed
Fix destruction ordering on shutdown.
cling is currently tearing down the text input before the Interpreter is destroyed. Leading to problems with pipes and file handles being close in the text input layer.
1 parent 79c5a90 commit 1b894b3

File tree

3 files changed

+48
-28
lines changed

3 files changed

+48
-28
lines changed

include/cling/UserInterface/UserInterface.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,24 @@ namespace cling {
2020
///
2121
class UserInterface {
2222
private:
23+
class TextInputHolder;
24+
std::unique_ptr<TextInputHolder> m_TextInput;
2325
std::unique_ptr<MetaProcessor> m_MetaProcessor;
2426

2527
///\brief Prints cling's startup logo
2628
///
2729
void PrintLogo();
2830
public:
29-
UserInterface(Interpreter& interp);
31+
UserInterface();
3032
~UserInterface();
3133

3234
MetaProcessor* getMetaProcessor() { return m_MetaProcessor.get(); }
3335

36+
///\brief Attach this instance to the given Interpreter.
37+
/// @param[in] Interp - The interpreter to attach to.
38+
///
39+
void attach(Interpreter& Interp);
40+
3441
///\brief Drives the interactive prompt talking to the user.
3542
/// @param[in] nologo - whether to show cling's welcome logo or not
3643
///

lib/UserInterface/UserInterface.cpp

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -46,57 +46,67 @@ namespace {
4646
return true;
4747
}
4848
};
49+
}
50+
51+
namespace cling {
4952

5053
///\brief Delays ~TextInput until after ~StreamReader and ~TerminalDisplay
5154
///
52-
class TextInputHolder {
55+
class UserInterface::TextInputHolder {
5356
textinput::StreamReader* m_Reader;
5457
textinput::TerminalDisplay* m_Display;
5558
textinput::TextInput m_Input;
5659

60+
class HistoryFile {
61+
llvm::SmallString<512> Path;
62+
63+
public:
64+
HistoryFile(const char* Env = "CLING_NOHISTORY",
65+
const char* Name = ".cling_history") {
66+
if (getenv(Env)) return;
67+
// History file is $HOME/.cling_history
68+
if (llvm::sys::path::home_directory(Path))
69+
llvm::sys::path::append(Path, Name);
70+
}
71+
const char* c_str() { return Path.empty() ? nullptr : Path.c_str(); }
72+
};
73+
5774
public:
58-
TextInputHolder(llvm::SmallString<512>& Hist)
75+
TextInputHolder(HistoryFile HistFile = HistoryFile())
5976
: m_Reader(textinput::StreamReader::Create()),
6077
m_Display(textinput::TerminalDisplay::Create()),
61-
m_Input(*m_Reader, *m_Display, Hist.empty() ? 0 : Hist.c_str()) {}
78+
m_Input(*m_Reader, *m_Display, HistFile.c_str()) {
79+
}
6280

6381
~TextInputHolder() {
6482
delete m_Reader;
6583
delete m_Display;
6684
}
6785

68-
textinput::TextInput* operator -> () { return &m_Input; }
86+
operator textinput::TextInput& () { return m_Input; }
6987
};
70-
}
7188

72-
namespace cling {
73-
74-
UserInterface::UserInterface(Interpreter& interp) {
75-
m_MetaProcessor.reset(new MetaProcessor(interp, cling::outs()));
89+
UserInterface::UserInterface() {
7690
llvm::install_fatal_error_handler(&CompilationException::throwingHandler);
7791
}
7892

79-
UserInterface::~UserInterface() {}
93+
UserInterface::~UserInterface() { llvm::remove_fatal_error_handler(); }
94+
95+
void UserInterface::attach(Interpreter& Interp) {
96+
m_MetaProcessor.reset(new MetaProcessor(Interp, cling::outs()));
97+
}
8098

8199
void UserInterface::runInteractively(bool nologo /* = false */) {
82-
if (!nologo) {
100+
assert(m_MetaProcessor.get() && "Not attached to an Interpreter.");
101+
if (!nologo)
83102
PrintLogo();
84-
}
85-
86-
llvm::SmallString<512> histfilePath;
87-
if (!getenv("CLING_NOHISTORY")) {
88-
// History file is $HOME/.cling_history
89-
if (llvm::sys::path::home_directory(histfilePath))
90-
llvm::sys::path::append(histfilePath, ".cling_history");
91-
}
92103

93-
TextInputHolder TI(histfilePath);
104+
m_TextInput.reset(new TextInputHolder);
105+
textinput::TextInput& TI = *m_TextInput;
94106

95107
// Inform text input about the code complete consumer
96108
// TextInput owns the TabCompletion.
97-
UITabCompletion* Completion =
98-
new UITabCompletion(m_MetaProcessor->getInterpreter());
99-
TI->SetCompletion(Completion);
109+
TI.SetCompletion(new UITabCompletion(m_MetaProcessor->getInterpreter()));
100110

101111
bool Done = false;
102112
std::string Line;
@@ -107,9 +117,9 @@ namespace cling {
107117
m_MetaProcessor->getOuts().flush();
108118
{
109119
MetaProcessor::MaybeRedirectOutputRAII RAII(*m_MetaProcessor);
110-
TI->SetPrompt(Prompt.c_str());
111-
Done = TI->ReadInput() == textinput::TextInput::kRREOF;
112-
TI->TakeInput(Line);
120+
TI.SetPrompt(Prompt.c_str());
121+
Done = TI.ReadInput() == textinput::TextInput::kRREOF;
122+
TI.TakeInput(Line);
113123
if (Done && Line.empty())
114124
break;
115125
}

tools/driver/cling.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ static int checkDiagErrors(clang::CompilerInstance* CI, unsigned* OutErrs = 0) {
5353

5454

5555
int main( int argc, char **argv ) {
56+
// Force the UserInterface to be destroyed last, so any file manipulation,
57+
// pipes, or dups are active for the entire process.
58+
cling::UserInterface Ui;
5659

5760
llvm::llvm_shutdown_obj shutdownTrigger;
5861

@@ -105,7 +108,7 @@ int main( int argc, char **argv ) {
105108
for (const std::string& Lib : Opts.LibsToLoad)
106109
Interp.loadFile(Lib);
107110

108-
cling::UserInterface Ui(Interp);
111+
Ui.attach(Interp);
109112
// If we are not interactive we're supposed to parse files
110113
if (!Opts.IsInteractive()) {
111114
for (const std::string &Input : Opts.Inputs) {

0 commit comments

Comments
 (0)