[Lldb-commits] [lldb] r250838 - Fix potential file i/o problem with python handles.

Zachary Turner via lldb-commits lldb-commits at lists.llvm.org
Tue Oct 20 10:38:50 PDT 2015


Author: zturner
Date: Tue Oct 20 12:38:49 2015
New Revision: 250838

URL: http://llvm.org/viewvc/llvm-project?rev=250838&view=rev
Log:
Fix potential file i/o problem with python handles.

Modified:
    lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
    lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
    lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
    lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h

Modified: lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp?rev=250838&r1=250837&r2=250838&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp (original)
+++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp Tue Oct 20 12:38:49 2015
@@ -605,6 +605,11 @@ PythonDictionary::CreateStructuredDictio
     return result;
 }
 
+PythonFile::PythonFile()
+    : PythonObject()
+{
+}
+
 PythonFile::PythonFile(File &file, const char *mode)
 {
     Reset(file, mode);

Modified: lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h?rev=250838&r1=250837&r2=250838&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h (original)
+++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h Tue Oct 20 12:38:49 2015
@@ -334,6 +334,7 @@ public:
 class PythonFile : public PythonObject
 {
   public:
+    PythonFile();
     PythonFile(File &file, const char *mode);
     PythonFile(const char *path, const char *mode);
     PythonFile(PyRefType type, PyObject *o);

Modified: lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp?rev=250838&r1=250837&r2=250838&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp (original)
+++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp Tue Oct 20 12:38:49 2015
@@ -612,7 +612,7 @@ ScriptInterpreterPython::EnterSession (u
                 // Flush the file before giving it to python to avoid interleaved output.
                 in_file.Flush();
 
-                m_saved_stdin = sys_module_dict.GetItemForKey(PythonString("stdin"));
+                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);
@@ -626,7 +626,7 @@ ScriptInterpreterPython::EnterSession (u
             // Flush the file before giving it to python to avoid interleaved output.
             out_file.Flush();
 
-            m_saved_stdout = sys_module_dict.GetItemForKey(PythonString("stdout"));
+            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);
@@ -641,7 +641,7 @@ ScriptInterpreterPython::EnterSession (u
             // Flush the file before giving it to python to avoid interleaved output.
             err_file.Flush();
 
-            m_saved_stderr = sys_module_dict.GetItemForKey(PythonString("stderr"));
+            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);
@@ -812,48 +812,55 @@ ScriptInterpreterPython::ExecuteOneLine
         FILE *in_file = input_file_sp->GetFile().GetStream();
         FILE *out_file = output_file_sp->GetFile().GetStream();
         FILE *err_file = error_file_sp->GetFile().GetStream();
-        Locker locker(this,
-                      ScriptInterpreterPython::Locker::AcquireLock |
-                      ScriptInterpreterPython::Locker::InitSession |
-                      (options.GetSetLLDBGlobals() ? ScriptInterpreterPython::Locker::InitGlobals : 0) |
-                      ((result && result->GetInteractive()) ? 0: Locker::NoSTDIN),
-                      ScriptInterpreterPython::Locker::FreeAcquiredLock |
-                      ScriptInterpreterPython::Locker::TearDownSession,
-                      in_file,
-                      out_file,
-                      err_file);
-        
         bool success = false;
-        
-        // Find the correct script interpreter dictionary in the main module.
-        PythonDictionary &session_dict = GetSessionDictionary ();
-        if (session_dict.IsValid())
         {
-            if (GetEmbeddedInterpreterModuleObjects ())
+            // WARNING!  It's imperative that this RAII scope be as tight as possible.  In particular, the
+            // scope must end *before* we try to join the read thread.  The reason for this is that a
+            // pre-requisite for joining the read thread is that we close the write handle (to break the
+            // pipe and cause it to wake up and exit).  But acquiring the GIL as below will redirect Python's
+            // stdio to use this same handle.  If we close the handle while Python is still using it, bad
+            // things will happen.
+            Locker locker(this,
+                          ScriptInterpreterPython::Locker::AcquireLock |
+                          ScriptInterpreterPython::Locker::InitSession |
+                          (options.GetSetLLDBGlobals() ? ScriptInterpreterPython::Locker::InitGlobals : 0) |
+                          ((result && result->GetInteractive()) ? 0: Locker::NoSTDIN),
+                          ScriptInterpreterPython::Locker::FreeAcquiredLock |
+                          ScriptInterpreterPython::Locker::TearDownSession,
+                          in_file,
+                          out_file,
+                          err_file);
+        
+            // Find the correct script interpreter dictionary in the main module.
+            PythonDictionary &session_dict = GetSessionDictionary ();
+            if (session_dict.IsValid())
             {
-                if (PyCallable_Check(m_run_one_line_function.get()))
+                if (GetEmbeddedInterpreterModuleObjects ())
                 {
-                    PythonObject pargs(PyRefType::Owned, Py_BuildValue("(Os)", session_dict.get(), command));
-                    if (pargs.IsValid())
+                    if (PyCallable_Check(m_run_one_line_function.get()))
                     {
-                        PythonObject return_value(PyRefType::Owned,
-                            PyObject_CallObject(m_run_one_line_function.get(), pargs.get()));
-                        if (return_value.IsValid())
-                            success = true;
-                        else if (options.GetMaskoutErrors() && PyErr_Occurred ())
+                        PythonObject pargs(PyRefType::Owned, Py_BuildValue("(Os)", session_dict.get(), command));
+                        if (pargs.IsValid())
                         {
-                            PyErr_Print();
-                            PyErr_Clear();
+                            PythonObject return_value(PyRefType::Owned,
+                                PyObject_CallObject(m_run_one_line_function.get(), pargs.get()));
+                            if (return_value.IsValid())
+                                success = true;
+                            else if (options.GetMaskoutErrors() && PyErr_Occurred ())
+                            {
+                                PyErr_Print();
+                                PyErr_Clear();
+                            }
                         }
                     }
                 }
             }
-        }
 
-        // Flush our output and error file handles
-        ::fflush (out_file);
-        if (out_file != err_file)
-            ::fflush (err_file);
+            // Flush our output and error file handles
+            ::fflush (out_file);
+            if (out_file != err_file)
+                ::fflush (err_file);
+        }
         
         if (join_read_thread)
         {

Modified: lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h?rev=250838&r1=250837&r2=250838&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h (original)
+++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h Tue Oct 20 12:38:49 2015
@@ -570,9 +570,9 @@ protected:
     bool
     GetEmbeddedInterpreterModuleObjects ();
 
-    PythonObject m_saved_stdin;
-    PythonObject m_saved_stdout;
-    PythonObject m_saved_stderr;
+    PythonFile m_saved_stdin;
+    PythonFile m_saved_stdout;
+    PythonFile m_saved_stderr;
     PythonObject m_main_module;
     PythonObject m_lldb_module;
     PythonDictionary m_session_dict;




More information about the lldb-commits mailing list