[Lldb-commits] [lldb] r156499 - in /lldb/branches/apple/python-GIL: include/lldb/Interpreter/ScriptInterpreterPython.h source/Interpreter/ScriptInterpreterPython.cpp

Filipe Cabecinhas me at filcab.net
Wed May 9 11:49:20 PDT 2012


Author: filcab
Date: Wed May  9 13:49:20 2012
New Revision: 156499

URL: http://llvm.org/viewvc/llvm-project?rev=156499&view=rev
Log:
Make ScriptInterpreterPython use the Python GIL mechanism.

Modified:
    lldb/branches/apple/python-GIL/include/lldb/Interpreter/ScriptInterpreterPython.h
    lldb/branches/apple/python-GIL/source/Interpreter/ScriptInterpreterPython.cpp

Modified: lldb/branches/apple/python-GIL/include/lldb/Interpreter/ScriptInterpreterPython.h
URL: http://llvm.org/viewvc/llvm-project/lldb/branches/apple/python-GIL/include/lldb/Interpreter/ScriptInterpreterPython.h?rev=156499&r1=156498&r2=156499&view=diff
==============================================================================
--- lldb/branches/apple/python-GIL/include/lldb/Interpreter/ScriptInterpreterPython.h (original)
+++ lldb/branches/apple/python-GIL/include/lldb/Interpreter/ScriptInterpreterPython.h Wed May  9 13:49:20 2012
@@ -228,10 +228,7 @@
                 FILE* wait_msg_handle = NULL);
         
     	~Locker ();
-    
-        static bool
-        CurrentThreadHasPythonLock ();
-        
+
 	private:
         
         bool
@@ -245,17 +242,14 @@
         
         bool
         DoTearDownSession ();
-        
-        static bool
-        TryGetPythonLock (uint32_t seconds_to_wait);
-        
+
         static void
         ReleasePythonLock ();
         
     	bool                     m_need_session;
-    	bool                     m_release_lock;
     	ScriptInterpreterPython *m_python_interpreter;
     	FILE*                    m_tmp_fh;
+        PyGILState_STATE         m_GILState;
 	};
 
     static size_t
@@ -278,7 +272,6 @@
     bool m_session_is_active;
     bool m_pty_slave_is_open;
     bool m_valid_session;
-                         
 };
 } // namespace lldb_private
 

Modified: lldb/branches/apple/python-GIL/source/Interpreter/ScriptInterpreterPython.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/branches/apple/python-GIL/source/Interpreter/ScriptInterpreterPython.cpp?rev=156499&r1=156498&r2=156499&view=diff
==============================================================================
--- lldb/branches/apple/python-GIL/source/Interpreter/ScriptInterpreterPython.cpp (original)
+++ lldb/branches/apple/python-GIL/source/Interpreter/ScriptInterpreterPython.cpp Wed May  9 13:49:20 2012
@@ -117,73 +117,18 @@
   return fflush (stream) || prev_fail ? EOF : 0;
 }
 
-static Predicate<lldb::tid_t> &
-PythonMutexPredicate ()
-{
-    static lldb_private::Predicate<lldb::tid_t> g_interpreter_is_running (LLDB_INVALID_THREAD_ID);
-    return g_interpreter_is_running;
-}
-
-bool
-ScriptInterpreterPython::Locker::CurrentThreadHasPythonLock ()
-{
-    TimeValue timeout;
-
-    timeout = TimeValue::Now();  // Don't wait any time.
-
-    return PythonMutexPredicate().WaitForValueEqualTo (Host::GetCurrentThreadID(), &timeout, NULL);
-}
-
-bool
-ScriptInterpreterPython::Locker::TryGetPythonLock (uint32_t seconds_to_wait)
-{
-    
-    TimeValue timeout;
-    
-    if (seconds_to_wait != UINT32_MAX)
-    {
-        timeout = TimeValue::Now();
-        timeout.OffsetWithSeconds (seconds_to_wait);
-    }
-    
-    return PythonMutexPredicate().WaitForValueEqualToAndSetValueTo (LLDB_INVALID_THREAD_ID, 
-                                                                    Host::GetCurrentThreadID(), &timeout, NULL);
-}
-
-void
-ScriptInterpreterPython::Locker::ReleasePythonLock ()
-{
-    PythonMutexPredicate().SetValue (LLDB_INVALID_THREAD_ID, eBroadcastAlways);
-}
-
 ScriptInterpreterPython::Locker::Locker (ScriptInterpreterPython *py_interpreter,
                                          uint16_t on_entry,
                                          uint16_t on_leave,
                                          FILE* wait_msg_handle) :
     m_need_session( (on_leave & TearDownSession) == TearDownSession ),
-    m_release_lock ( false ), // decide in constructor body
     m_python_interpreter(py_interpreter),
     m_tmp_fh(wait_msg_handle)
 {
     if (m_python_interpreter && !m_tmp_fh)
         m_tmp_fh = (m_python_interpreter->m_dbg_stdout ? m_python_interpreter->m_dbg_stdout : stdout);
-    
-    if ( (on_entry & AcquireLock) == AcquireLock )
-    {
-        if (CurrentThreadHasPythonLock())
-        {
-            if ( (on_leave & FreeLock) == FreeLock )
-                m_release_lock = true;
-        }
-        else
-        {
-            DoAcquireLock();
-            if ( (on_leave & FreeLock) == FreeLock )
-                m_release_lock = true;
-            if ( (on_leave & FreeAcquiredLock) == FreeAcquiredLock )
-                m_release_lock = true;
-        }
-    }
+
+    DoAcquireLock();
     if ( (on_entry & InitSession) == InitSession )
         DoInitSession();
 }
@@ -191,13 +136,10 @@
 bool
 ScriptInterpreterPython::Locker::DoAcquireLock()
 {
-    if (!CurrentThreadHasPythonLock())
-    {
-        while (!TryGetPythonLock (1))
-            if (m_tmp_fh)
-                fprintf (m_tmp_fh, 
-                         "Python interpreter locked on another thread; waiting to acquire lock...\n");
-    }
+    LogSP log (lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_SCRIPT | LIBLLDB_LOG_VERBOSE));
+    m_GILState = PyGILState_Ensure();
+    if (log)
+        log->Printf("Ensured PyGILState. Previous state = %slocked\n", m_GILState == PyGILState_UNLOCKED ? "un" : "");
     return true;
 }
 
@@ -213,7 +155,10 @@
 bool
 ScriptInterpreterPython::Locker::DoFreeLock()
 {
-    ReleasePythonLock ();
+    LogSP log (lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_SCRIPT | LIBLLDB_LOG_VERBOSE));
+    if (log)
+        log->Printf("Releasing PyGILState. Returning to state = %slocked\n", m_GILState == PyGILState_UNLOCKED ? "un" : "");
+    PyGILState_Release(m_GILState);
     return true;
 }
 
@@ -230,8 +175,7 @@
 {
     if (m_need_session)
         DoTearDownSession();
-    if (m_release_lock)
-        DoFreeLock();
+    DoFreeLock();
 }
 
 class ForceDisableSyntheticChildren
@@ -1526,7 +1470,13 @@
     if (pty_slave_name != NULL)
     {
         StreamString run_string;
-        
+
+        // Ensure we have the GIL before running any Python code.
+        // Since we're only running a few one-liners and then dropping to the interpreter.
+        // (which will release the GIL when needed), we can just release the GIL after finishing our work.
+        // If finer-grained locking is desirable, we can lock and unlock the GIL only when calling a python function.
+        PyGILState_STATE gstate = PyGILState_Ensure();
+
         run_string.Printf ("run_one_line (%s, 'save_stderr = sys.stderr')", script_interpreter->m_dictionary_name.c_str());
         PyRun_SimpleString (run_string.GetData());
         run_string.Clear ();
@@ -1546,36 +1496,27 @@
 
         // The following call drops into the embedded interpreter loop and stays there until the
         // user chooses to exit from the Python interpreter.
+        // This embedded interpreter will, as any Python code that performs I/O, unlock the GIL before
+        // a system call that can hang, and lock it when the syscall has returned.
 
-        // When in the embedded interpreter, the user can call arbitrary system and Python stuff, which may require
-        // the ability to run multi-threaded stuff, so we need to surround the call to the embedded interpreter with
-        // calls to Py_BEGIN_ALLOW_THREADS and Py_END_ALLOW_THREADS.
-
-        // We ALSO need to surround the call to the embedded interpreter with calls to PyGILState_Ensure and 
-        // PyGILState_Release.  This is because this embedded interpreter is being run on a DIFFERENT THREAD than
-        // the thread on which the call to Py_Initialize (and PyEval_InitThreads) was called.  Those initializations 
-        // called PyGILState_Ensure on *that* thread, but it also needs to be called on *this* thread.  Otherwise,
-        // if the user calls Python code that does threading stuff, the interpreter state will be off, and things could
-        // hang (it's happened before).
+        // We need to surround the call to the embedded interpreter with calls to PyGILState_Ensure and 
+        // PyGILState_Release.  This is because Python has a global lock which must be held whenever we want
+        // to touch any Python objects. Otherwise, if the user calls Python code, the interpreter state will be off,
+        // and things could hang (it's happened before).
 
-        Py_BEGIN_ALLOW_THREADS
-        PyGILState_STATE gstate = PyGILState_Ensure();
-        
         run_string.Printf ("run_python_interpreter (%s)", script_interpreter->m_dictionary_name.c_str());
         PyRun_SimpleString (run_string.GetData());
         run_string.Clear ();
-        
-        PyGILState_Release (gstate);
-        Py_END_ALLOW_THREADS
-        
+
         run_string.Printf ("run_one_line (%s, 'sys.stdin = save_stdin')", script_interpreter->m_dictionary_name.c_str());
         PyRun_SimpleString (run_string.GetData());
         run_string.Clear();
-        
+
         run_string.Printf ("run_one_line (%s, 'sys.stderr = save_stderr')", script_interpreter->m_dictionary_name.c_str());
         PyRun_SimpleString (run_string.GetData());
         run_string.Clear();
-        
+
+        PyGILState_Release (gstate);
     }
     
     if (script_interpreter->m_embedded_thread_input_reader_sp)
@@ -1924,7 +1865,18 @@
     TerminalState stdin_tty_state;
     stdin_tty_state.Save(STDIN_FILENO, false);
 
-    PyEval_InitThreads ();
+    PyGILState_STATE gstate;
+    LogSP log (lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_SCRIPT | LIBLLDB_LOG_VERBOSE));
+    bool threads_already_initialized = false;
+    if (PyEval_ThreadsInitialized ()) {
+        gstate = PyGILState_Ensure ();
+        if (log)
+            log->Printf("Ensured PyGILState. Previous state = %slocked\n", gstate == PyGILState_UNLOCKED ? "un" : "");
+        threads_already_initialized = true;
+    } else {
+        // InitThreads acquires the GIL if it hasn't been called before.
+        PyEval_InitThreads ();
+    }
     Py_InitializeEx (0);
 
     // Initialize SWIG after setting up python
@@ -1967,6 +1919,15 @@
 
     PyRun_SimpleString ("sys.dont_write_bytecode = 1; import lldb.embedded_interpreter; from lldb.embedded_interpreter import run_python_interpreter; from lldb.embedded_interpreter import run_one_line; from termios import *");
 
+    if (threads_already_initialized) {
+        if (log)
+            log->Printf("Releasing PyGILState. Returning to state = %slocked\n", gstate == PyGILState_UNLOCKED ? "un" : "");
+        PyGILState_Release (gstate);
+    } else {
+        // We initialized the threads in this function, just unlock the GIL.
+        PyEval_SaveThread();
+    }
+
     stdin_tty_state.Restore();
 }
 





More information about the lldb-commits mailing list