clayborg created this revision.
clayborg added a reviewer: zturner.
clayborg added a subscriber: lldb-commits.
After the Python 3 fixes the interactive python interpreter stopped working in
Xcode. The issue was the python interpreter tried to adopt the top IOHandler
file handles, but it failed due to lldb_private::File objects failing to copy
themselves into another lldb_private::File because they contained "FILE *" file
stream pointer, not just an integer file descriptor, and File::Duplicate()
didn't handle duplicating "FILE *". But we really don't want files duplicating
themselves each time we run a python expression, we just want to use the files.
The fix involves removing File::Duplicate() since we shouldn't be using this
and duplicating files. It also removes File's assignment operator and copy
constructor. It then fixes the python interpreter to correctly adopt the file
handles from the top IOHandler.
http://reviews.llvm.org/D18057
Files:
include/lldb/Host/File.h
source/Host/common/File.cpp
source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
Index: source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
===================================================================
--- source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
+++ source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
@@ -581,6 +581,9 @@
bool
GetEmbeddedInterpreterModuleObjects ();
+ bool
+ SetStdHandle(File &file, const char *py_name, PythonFile &save_file, const char *mode);
+
PythonFile m_saved_stdin;
PythonFile m_saved_stdout;
PythonFile m_saved_stderr;
Index: source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===================================================================
--- source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -547,6 +547,27 @@
}
bool
+ScriptInterpreterPython::SetStdHandle(File &file, const char *py_name, PythonFile &save_file, const char *mode)
+{
+ if (file.IsValid())
+ {
+ // Flush the file before giving it to python to avoid interleaved output.
+ file.Flush();
+
+ PythonDictionary &sys_module_dict = GetSysModuleDictionary();
+
+ save_file = sys_module_dict.GetItemForKey(PythonString(py_name)).AsType<PythonFile>();
+
+ PythonFile new_file(file, mode);
+ sys_module_dict.SetItemForKey(PythonString(py_name), new_file);
+ return true;
+ }
+ else
+ save_file.Reset();
+ return false;
+}
+
+bool
ScriptInterpreterPython::EnterSession (uint16_t on_entry_flags,
FILE *in,
FILE *out,
@@ -604,54 +625,31 @@
if (!in_file.IsValid() || !out_file.IsValid() || !err_file.IsValid())
m_interpreter.GetDebugger().AdoptTopIOHandlerFilesIfInvalid (in_sp, out_sp, err_sp);
- m_saved_stdin.Reset();
- if ((on_entry_flags & Locker::NoSTDIN) == 0)
+ if (on_entry_flags & Locker::NoSTDIN)
{
- // STDIN is enabled
- if (!in_file.IsValid() && in_sp)
- in_file = in_sp->GetFile();
- if (in_file.IsValid())
+ m_saved_stdin.Reset();
+ }
+ else
+ {
+ if (!SetStdHandle(in_file, "stdin", m_saved_stdin, "r"))
{
- // Flush the file before giving it to python to avoid interleaved output.
- in_file.Flush();
-
- m_saved_stdin = sys_module_dict.GetItemForKey(PythonString("stdin")).AsType<PythonFile>();
- // This call can deadlock your process if the file is locked
- PythonFile new_file(in_file, "r");
- sys_module_dict.SetItemForKey (PythonString("stdin"), new_file);
+ if (in_sp)
+ SetStdHandle(in_sp->GetFile(), "stdin", m_saved_stdin, "r");
}
}
- if (!out_file.IsValid() && out_sp)
- out_file = out_sp->GetFile();
- if (out_file.IsValid())
+ if (!SetStdHandle(out_file, "stdout", m_saved_stdout, "w"))
{
- // Flush the file before giving it to python to avoid interleaved output.
- out_file.Flush();
-
- m_saved_stdout = sys_module_dict.GetItemForKey(PythonString("stdout")).AsType<PythonFile>();
-
- PythonFile new_file(out_file, "w");
- sys_module_dict.SetItemForKey (PythonString("stdout"), new_file);
+ if (out_sp)
+ SetStdHandle(out_sp->GetFile(), "stdout", m_saved_stdout, "w");
}
- else
- m_saved_stdout.Reset();
- if (!err_file.IsValid() && err_sp)
- err_file = err_sp->GetFile();
- if (err_file.IsValid())
+ if (!SetStdHandle(err_file, "stderr", m_saved_stderr, "w"))
{
- // Flush the file before giving it to python to avoid interleaved output.
- err_file.Flush();
-
- m_saved_stderr = sys_module_dict.GetItemForKey(PythonString("stderr")).AsType<PythonFile>();
-
- PythonFile new_file(err_file, "w");
- sys_module_dict.SetItemForKey (PythonString("stderr"), new_file);
+ if (err_sp)
+ SetStdHandle(err_sp->GetFile(), "stderr", m_saved_stderr, "w");
}
- else
- m_saved_stderr.Reset();
}
if (PyErr_Occurred())
Index: source/Host/common/File.cpp
===================================================================
--- source/Host/common/File.cpp
+++ source/Host/common/File.cpp
@@ -109,27 +109,6 @@
}
}
-File::File (const File &rhs) :
- IOObject(eFDTypeFile, false),
- m_descriptor (kInvalidDescriptor),
- m_stream (kInvalidStream),
- m_options (0),
- m_own_stream (false),
- m_is_interactive (eLazyBoolCalculate),
- m_is_real_terminal (eLazyBoolCalculate)
-{
- Duplicate (rhs);
-}
-
-
-File &
-File::operator = (const File &rhs)
-{
- if (this != &rhs)
- Duplicate (rhs);
- return *this;
-}
-
File::~File()
{
Close ();
@@ -215,7 +194,6 @@
return m_stream;
}
-
void
File::SetStream (FILE *fh, bool transfer_ownership)
{
@@ -226,35 +204,6 @@
}
Error
-File::Duplicate (const File &rhs)
-{
- Error error;
- if (IsValid ())
- Close();
-
- if (rhs.DescriptorIsValid())
- {
-#ifdef _WIN32
- m_descriptor = ::_dup(rhs.GetDescriptor());
-#else
- m_descriptor = dup(rhs.GetDescriptor());
-#endif
- if (!DescriptorIsValid())
- error.SetErrorToErrno();
- else
- {
- m_options = rhs.m_options;
- m_should_close_fd = true;
- }
- }
- else
- {
- error.SetErrorString ("invalid file to duplicate");
- }
- return error;
-}
-
-Error
File::Open (const char *path, uint32_t options, uint32_t permissions)
{
Error error;
Index: include/lldb/Host/File.h
===================================================================
--- include/lldb/Host/File.h
+++ include/lldb/Host/File.h
@@ -74,8 +74,6 @@
{
}
- File (const File &rhs);
-
//------------------------------------------------------------------
/// Constructor with path.
///
@@ -138,9 +136,6 @@
//------------------------------------------------------------------
~File() override;
- File &
- operator= (const File &rhs);
-
bool
IsValid() const override
{
@@ -226,9 +221,6 @@
void
Clear ();
- Error
- Duplicate (const File &rhs);
-
int
GetDescriptor() const;
@@ -557,6 +549,9 @@
LazyBool m_is_interactive;
LazyBool m_is_real_terminal;
LazyBool m_supports_colors;
+
+private:
+ DISALLOW_COPY_AND_ASSIGN(File);
};
} // namespace lldb_private
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
